[dpdk-dev] [RFC PATCH] detect missing experimental tags in headers

[dpdk-dev] [RFC PATCH] detect missing experimental tags in headers

[David Marchand]

Add a little script to get the symbols of a given section looking at a
library map file, then, on installation, inspect the sources headers to
check that the declaration of the EXPERIMENTAL symbols are prefixed with
a __rte_experimental tag.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---

For now, the check is not fatal, since this is just a draft.
Finding out the right headers could be improved.
We could also move this check to checkpatches so that it is enforced by
maintainers.

---
 buildtools/map-list-symbol.sh | 23 +++++++++++++++++++++++
 mk/rte.lib.mk                 | 27 ++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100755 buildtools/map-list-symbol.sh

diff --git a/buildtools/map-list-symbol.sh b/buildtools/map-list-symbol.sh
new file mode 100755
index 0000000..e4635df
--- /dev/null
+++ b/buildtools/map-list-symbol.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 David Marchand <david.marchand@redhat.com>
+
+section=${1:-}
+shift
+
+for file in $@; do
+	cat "$file" |awk '
+	BEGIN { in_sec = 0; }
+	/^.*{/ {
+		if ("'$section'" == "all" || $1 == "'$section'") {
+			in_sec = 1;
+		}
+	}
+	/.*}/ { in_sec = 0; }
+	/^[^}].*[^:*];/ {
+		if (in_sec == 1) {
+			gsub(";","");
+			print $1;
+		}
+	}'
+done
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index c696a21..fdd9059 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -31,7 +31,7 @@ endif
 
 
 _BUILD = $(LIB)
-PREINSTALL = $(SYMLINK-FILES-y)
+PREINSTALL = $(SYMLINK-FILES-y) check_headers_experimental
 _INSTALL = $(INSTALL-FILES-y) $(RTE_OUTPUT)/lib/$(LIB)
 _CLEAN = doclean
 
@@ -127,6 +127,31 @@ $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
 endif
 
 #
+# __rte_experimental inspection
+#
+check_headers_experimental:
+	@list_symbols=""; \
+	for sym in $$($(RTE_SDK)/buildtools/map-list-symbol.sh EXPERIMENTAL $(SRCDIR)/$(EXPORT_MAP)); do \
+		srcdir=$(SRCDIR); \
+		if [ "$${srcdir%%linuxapp/eal}" != "$$srcdir" ]; then \
+			srcdir="$$srcdir $${srcdir%%linuxapp/eal}common"; \
+		fi; \
+		files=$$(find $$srcdir -name '*.h' |xargs grep -l $$sym); \
+		if [ -z "$$files" ]; then \
+			list_symbols="$$list_symbols $$sym"; \
+			continue; \
+		fi; \
+		if ! grep -Pzoha "[^)/;}]+\s*$$sym\s*\(" $$files |grep -q __rte_experimental; then \
+			list_symbols="$$list_symbols $$sym"; \
+		fi; \
+	done; \
+	if [ -n "$$list_symbols" ]; then \
+		for sym in $$list_symbols; do \
+			echo "ERROR: $$sym is not marked as experimental in this library headers"; \
+		done; \
+	fi
+
+#
 # install lib in $(RTE_OUTPUT)/lib
 #
 $(RTE_OUTPUT)/lib/$(LIB): $(LIB)
-- 
1.8.3.1

Re: [dpdk-dev] [RFC PATCH] detect missing experimental tags in headers

[David Marchand]

On Wed, Dec 19, 2018 at 2:45 PM David Marchand <david.marchand@redhat.com>
wrote:

> Add a little script to get the symbols of a given section looking at a
> library map file, then, on installation, inspect the sources headers to
> check that the declaration of the EXPERIMENTAL symbols are prefixed with
> a __rte_experimental tag.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>
> For now, the check is not fatal, since this is just a draft.
> Finding out the right headers could be improved.
> We could also move this check to checkpatches so that it is enforced by
> maintainers.
>

Never got any review/comment, if you try with current origin/master, there
is a little update to do for the eal directory hack:
-               if [ "$${srcdir%%linuxapp/eal}" != "$$srcdir" ]; then \
-                       srcdir="$$srcdir $${srcdir%%linuxapp/eal}common"; \
+               if [ "$${srcdir%%/eal}" != "$$srcdir" ]; then \
+                       srcdir="$$srcdir $${srcdir%/*/eal}/common"; \

I did not catch false positive so far.
Surely there could be problems with the grep regexp, but if we have this in
the build framework, people will get errors before submitting.



Btw, I caught some warnings in the following components.
CC relevant maintainers, can you look at this ?


== Build lib/librte_bbdev
ERROR: rte_bbdev_devices is not marked as experimental in this library
headers

This symbol is in the map file but is neither marked in header nor in the
code.
This is an array, does it qualify as an experimental API?

== Build lib/librte_cryptodev
ERROR: rte_crypto_asym_op_strings is not marked as experimental in this
library headers
ERROR: rte_crypto_asym_xform_strings is not marked as experimental in this
library headers

Idem.


== Build lib/librte_vhost
ERROR: rte_vhost_va_from_guest_pa is not marked as experimental in this
library headers

This symbol is in the map file but is neither marked in header nor in the
code.
Here, this is an inline, not sure the experimental tag is that useful
(except documenting it is experimental?).


-- 
David Marchand

Re: [dpdk-dev] [RFC PATCH] detect missing experimental tags in headers

[David Marchand]

On Wed, Dec 19, 2018 at 2:45 PM David Marchand <david.marchand@redhat.com>
wrote:

> Add a little script to get the symbols of a given section looking at a
> library map file, then, on installation, inspect the sources headers to
> check that the declaration of the EXPERIMENTAL symbols are prefixed with
> a __rte_experimental tag.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>
> For now, the check is not fatal, since this is just a draft.
> Finding out the right headers could be improved.
> We could also move this check to checkpatches so that it is enforced by
> maintainers.
>

Never got any review/comment, if you try with current origin/master, there
is a little update to do for the eal directory hack:
-               if [ "$${srcdir%%linuxapp/eal}" != "$$srcdir" ]; then \
-                       srcdir="$$srcdir $${srcdir%%linuxapp/eal}common"; \
+               if [ "$${srcdir%%/eal}" != "$$srcdir" ]; then \
+                       srcdir="$$srcdir $${srcdir%/*/eal}/common"; \

I did not catch false positive so far.
Surely there could be problems with the grep regexp, but if we have this in
the build framework, people will get errors before submitting.



Btw, I caught some warnings in the following components.
CC relevant maintainers, can you look at this ?


== Build lib/librte_bbdev
ERROR: rte_bbdev_devices is not marked as experimental in this library
headers

This symbol is in the map file but is neither marked in header nor in the
code.
This is an array, does it qualify as an experimental API?

== Build lib/librte_cryptodev
ERROR: rte_crypto_asym_op_strings is not marked as experimental in this
library headers
ERROR: rte_crypto_asym_xform_strings is not marked as experimental in this
library headers

Idem.


== Build lib/librte_vhost
ERROR: rte_vhost_va_from_guest_pa is not marked as experimental in this
library headers

This symbol is in the map file but is neither marked in header nor in the
code.
Here, this is an inline, not sure the experimental tag is that useful
(except documenting it is experimental?).


-- 
David Marchand