[RFC] detect missing experimental tags in headers
Checks
Commit Message
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
Comments
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?).
new file mode 100755
@@ -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
@@ -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)