[dpdk-dev] [PATCH] devtools: add script to verify map files

[dpdk-dev] [PATCH] devtools: add script to verify map files

[Pavan Nikhilesh]

This script checks for the symbols specified in the map files against
the symbols present in the map file directory.
Currently, the script checks for map files in drivers and lib directory.

Example:
./devtools/check-map.py

The following symbols are unused :

Map file : /home/pavan/Work/clean/dpdk/drivers/mempool/dpaa/...
['rte_dpaa_pool_table']

Map file : /home/pavan/Work/clean/dpdk/drivers/bus/fslmc/...
['qbman_get_version', 'qbman_swp_send_multiple']

...

Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
---
 devtools/check-map.py | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100755 devtools/check-map.py

diff --git a/devtools/check-map.py b/devtools/check-map.py
new file mode 100755
index 000000000..a508469d6
--- /dev/null
+++ b/devtools/check-map.py
@@ -0,0 +1,56 @@
+#! /usr/bin/env python
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2017 Cavium, Inc
+#
+
+import subprocess
+import re
+import os
+from glob import glob
+
+paths = ['drivers', 'lib']
+map_files = []
+
+for path in paths:
+    map_files = map_files + [y
+                             for x in os.walk(os.environ['PWD'] + '/' + path)
+                             for y in glob(os.path.join(x[0],
+                                                        '*.map'))]
+
+print("\nThe following symbols are unused : \n")
+for map_file in map_files:
+    path = os.path.dirname(os.path.abspath(map_file))
+    with open(map_file) as f:
+        map_content = [line.strip() for line in f]
+    # Filter empty lines.
+    map_content = list(filter(None, map_content))
+    regex = re.compile(
+            r"""
+            \bDPDK.*\b |
+            \bglobal.*\b |
+            \blocal.*\b | \}.* |
+            \bEXPERIMENTAL\b |
+            \bper_lcore.*\b""",
+            flags=re.I | re.X | re.VERBOSE)
+    filtered_map = filter(lambda i: not regex.search(i), map_content)
+    filtered_map = [i for i in map_content if not regex.search(i)]
+    filtered_map = [s.replace(';', '') for s in filtered_map]
+    # Filter out known lines.
+    if not filtered_map:
+        continue
+    not_found = []
+    # Gerp in the map file directory for each symbol
+    for map_symbol in filtered_map:
+        try:
+            op = subprocess.check_output(['grep', '-nr', '--exclude',
+                                          os.path.basename(map_file),
+                                          map_symbol,
+                                          path])
+        except subprocess.CalledProcessError as ex:
+            not_found.append(map_symbol)
+            rc = ex.returncode
+            if rc != 1:
+                raise
+    if not_found:
+        print("Map file : %s" % map_file)
+        print("%s\n" % not_found)
-- 
2.16.1

Re: [dpdk-dev] [PATCH] devtools: add script to verify map files

[Ferruh Yigit]

On 2/12/2018 4:13 PM, Pavan Nikhilesh wrote:
> This script checks for the symbols specified in the map files against
> the symbols present in the map file directory.
> Currently, the script checks for map files in drivers and lib directory.
> 
> Example:
> ./devtools/check-map.py
> 
> The following symbols are unused :
> 
> Map file : /home/pavan/Work/clean/dpdk/drivers/mempool/dpaa/...
> ['rte_dpaa_pool_table']
> 
> Map file : /home/pavan/Work/clean/dpdk/drivers/bus/fslmc/...
> ['qbman_get_version', 'qbman_swp_send_multiple']
> 
> ...
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>

+1 to improve our tools.

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH] devtools: add script to verify map files

[Thomas Monjalon]

Sorry for having missed this patch during so long.

13/02/2018 11:48, Ferruh Yigit:
> On 2/12/2018 4:13 PM, Pavan Nikhilesh wrote:
> > This script checks for the symbols specified in the map files against
> > the symbols present in the map file directory.
> > Currently, the script checks for map files in drivers and lib directory.
> > 
> > Example:
> > ./devtools/check-map.py
> > 
> > The following symbols are unused :
> > 
> > Map file : /home/pavan/Work/clean/dpdk/drivers/mempool/dpaa/...
> > ['rte_dpaa_pool_table']
> > 
> > Map file : /home/pavan/Work/clean/dpdk/drivers/bus/fslmc/...
> > ['qbman_get_version', 'qbman_swp_send_multiple']
> > 
> > ...
> > 
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> 
> +1 to improve our tools.
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

I agree we must have more tools.

I think this check can be a lot simpler as a shell script,
instead of Python. It does not need the list features of Python.

I've run it and I see it does not catch symbols which are renamed
with a prefix. Example:
	vfio_get_group_fd became rte_vfio_get_group_fd

[dpdk-dev] [PATCH v2] devtools: check orphan symbols in map files

[Thomas Monjalon]

The script check-symbol-maps.sh finds the symbols exported
in a map file but not referenced in the codebase.

Suggested-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v2:
    - rewrite Python script from Pavan in a smaller shell script
    - check symbol as whole word (-w)
    - print one symbol per line
---
 MAINTAINERS                   |  1 +
 devtools/check-symbol-maps.sh | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100755 devtools/check-symbol-maps.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index e56c72687..437777a22 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -81,6 +81,7 @@ F: devtools/check-dup-includes.sh
 F: devtools/check-maintainers.sh
 F: devtools/check-git-log.sh
 F: devtools/check-includes.sh
+F: devtools/check-symbol-maps.sh
 F: devtools/checkpatches.sh
 F: devtools/get-maintainer.sh
 F: devtools/git-log-fixes.sh
diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
new file mode 100755
index 000000000..e137d48a4
--- /dev/null
+++ b/devtools/check-symbol-maps.sh
@@ -0,0 +1,30 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2018 Mellanox Technologies, Ltd
+
+cd $(dirname $0)/..
+
+# speed up by ignoring Unicode details
+export LC_ALL=C
+
+find_orphan_symbols ()
+{
+    for map in $(find lib drivers -name '*.map') ; do
+        for sym in $(sed -rn 's,^([^}]*_.*);,\1,p' $map) ; do
+            if echo $sym | grep -q '^per_lcore_' ; then
+                continue
+            fi
+            if ! grep -q -r --exclude=$(basename $map) \
+                    -w $sym $(dirname $map) ; then
+                echo "$map: $sym"
+            fi
+        done
+    done
+}
+
+orphan_symbols=$(find_orphan_symbols)
+if [ -n "$orphan_symbols" ] ; then
+    echo "Found only in symbol map file:"
+    echo "$orphan_symbols" | sed 's,^,\t,'
+    exit 1
+fi
-- 
2.16.2

Re: [dpdk-dev] [PATCH] devtools: add script to verify map files

[Bruce Richardson]

On Sun, May 27, 2018 at 10:50:58PM +0200, Thomas Monjalon wrote:
> Sorry for having missed this patch during so long.
> 
> 13/02/2018 11:48, Ferruh Yigit:
> > On 2/12/2018 4:13 PM, Pavan Nikhilesh wrote:
> > > This script checks for the symbols specified in the map files against
> > > the symbols present in the map file directory.
> > > Currently, the script checks for map files in drivers and lib directory.
> > > 
> > > Example:
> > > ./devtools/check-map.py
> > > 
> > > The following symbols are unused :
> > > 
> > > Map file : /home/pavan/Work/clean/dpdk/drivers/mempool/dpaa/...
> > > ['rte_dpaa_pool_table']
> > > 
> > > Map file : /home/pavan/Work/clean/dpdk/drivers/bus/fslmc/...
> > > ['qbman_get_version', 'qbman_swp_send_multiple']
> > > 
> > > ...
> > > 
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > 
> > +1 to improve our tools.
> > 
> > Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> I agree we must have more tools.
> 
> I think this check can be a lot simpler as a shell script,
> instead of Python. It does not need the list features of Python.
> 

While the shell version can be shorter, I always find python scripts to be
far easier to read, understand and therefore maintain than shell scripts.
While power-users of shell, like yourself, Thomas, can come up with some
amazing stuff done in shell, it tends to make them very hard to follow for
us mere mortals.

/Bruce

Re: [dpdk-dev] [PATCH] devtools: add script to verify map files

[Thomas Monjalon]

28/05/2018 11:21, Bruce Richardson:
> On Sun, May 27, 2018 at 10:50:58PM +0200, Thomas Monjalon wrote:
> > Sorry for having missed this patch during so long.
> > 
> > 13/02/2018 11:48, Ferruh Yigit:
> > > On 2/12/2018 4:13 PM, Pavan Nikhilesh wrote:
> > > > This script checks for the symbols specified in the map files against
> > > > the symbols present in the map file directory.
> > > > Currently, the script checks for map files in drivers and lib directory.
> > > > 
> > > > Example:
> > > > ./devtools/check-map.py
> > > > 
> > > > The following symbols are unused :
> > > > 
> > > > Map file : /home/pavan/Work/clean/dpdk/drivers/mempool/dpaa/...
> > > > ['rte_dpaa_pool_table']
> > > > 
> > > > Map file : /home/pavan/Work/clean/dpdk/drivers/bus/fslmc/...
> > > > ['qbman_get_version', 'qbman_swp_send_multiple']
> > > > 
> > > > ...
> > > > 
> > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > 
> > > +1 to improve our tools.
> > > 
> > > Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > 
> > I agree we must have more tools.
> > 
> > I think this check can be a lot simpler as a shell script,
> > instead of Python. It does not need the list features of Python.
> > 
> 
> While the shell version can be shorter, I always find python scripts to be
> far easier to read, understand and therefore maintain than shell scripts.
> While power-users of shell, like yourself, Thomas, can come up with some
> amazing stuff done in shell, it tends to make them very hard to follow for
> us mere mortals.

Some processing are simpler in Python.
But here, the python script is mostly calling some shell commands and
filtering like grep.

For comparison, python version:
	http://dpdk.org/ml/archives/dev/2018-February/090772.html
shell version:
	http://dpdk.org/ml/archives/dev/2018-May/102993.html

Re: [dpdk-dev] [PATCH] devtools: add script to verify map files

[Bruce Richardson]

On Mon, May 28, 2018 at 11:31:31AM +0200, Thomas Monjalon wrote:
> 28/05/2018 11:21, Bruce Richardson:
> > On Sun, May 27, 2018 at 10:50:58PM +0200, Thomas Monjalon wrote:
> > > Sorry for having missed this patch during so long.
> > > 
> > > 13/02/2018 11:48, Ferruh Yigit:
> > > > On 2/12/2018 4:13 PM, Pavan Nikhilesh wrote:
> > > > > This script checks for the symbols specified in the map files against
> > > > > the symbols present in the map file directory.
> > > > > Currently, the script checks for map files in drivers and lib directory.
> > > > > 
> > > > > Example:
> > > > > ./devtools/check-map.py
> > > > > 
> > > > > The following symbols are unused :
> > > > > 
> > > > > Map file : /home/pavan/Work/clean/dpdk/drivers/mempool/dpaa/...
> > > > > ['rte_dpaa_pool_table']
> > > > > 
> > > > > Map file : /home/pavan/Work/clean/dpdk/drivers/bus/fslmc/...
> > > > > ['qbman_get_version', 'qbman_swp_send_multiple']
> > > > > 
> > > > > ...
> > > > > 
> > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > > 
> > > > +1 to improve our tools.
> > > > 
> > > > Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > 
> > > I agree we must have more tools.
> > > 
> > > I think this check can be a lot simpler as a shell script,
> > > instead of Python. It does not need the list features of Python.
> > > 
> > 
> > While the shell version can be shorter, I always find python scripts to be
> > far easier to read, understand and therefore maintain than shell scripts.
> > While power-users of shell, like yourself, Thomas, can come up with some
> > amazing stuff done in shell, it tends to make them very hard to follow for
> > us mere mortals.
> 
> Some processing are simpler in Python.
> But here, the python script is mostly calling some shell commands and
> filtering like grep.
> 
> For comparison, python version:
> 	http://dpdk.org/ml/archives/dev/2018-February/090772.html
> shell version:
> 	http://dpdk.org/ml/archives/dev/2018-May/102993.html
> 

Well, the python script does have a call to grep in it, true. I'm not going
to have a problem either way, the python version still reads a little
easier, but the shell version is shorter and looks more natural, I suppose.
I'm happy enough to go along with whatever others (who may care) think.

/Bruce

Re: [dpdk-dev] [PATCH v2] devtools: check orphan symbols in map files

[Thomas Monjalon]

27/05/2018 23:54, Thomas Monjalon:
> The script check-symbol-maps.sh finds the symbols exported
> in a map file but not referenced in the codebase.
> 
> Suggested-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v2:
>     - rewrite Python script from Pavan in a smaller shell script
>     - check symbol as whole word (-w)
>     - print one symbol per line
> ---
>  MAINTAINERS                   |  1 +
>  devtools/check-symbol-maps.sh | 30 ++++++++++++++++++++++++++++++

Applied