On Thu, Jan 28, 2021 at 12:20 PM Bruce Richardson
[off-list ref] wrote:

quoted

If the compiler has neither error or diagnose_if support, an internal
API can be called without ALLOW_INTERNAL_API.
I prefer a build error complaining on an unknown attribute rather than
silence a check.

I.e.

#ifndef ALLOW_INTERNAL_API

#if __has_attribute(diagnose_if) /* For clang */
#define __rte_internal \
__attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
section(".text.internal")))

#else
#define __rte_internal \
__attribute__((error("Symbol is not public ABI"), \
section(".text.internal")))

#endif

#else /* ALLOW_INTERNAL_API */

#define __rte_internal \
section(".text.internal")))

#endif

Would this not mean that if someone was using a compiler that supported
neither that they could never include any header file which contained any
internal functions? I'd rather err on the side of allowing this, on the
basis that the symbol should be already documented as internal and this is
only an additional sanity check.

- Still not a fan.
We will never know about those compilers behavior, like how we caught
the clang issue and found an alternative.


- I just caught a build error with RHEL7 gcc:

[1/2127] Compiling C object
lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o
FAILED: lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o
ccache cc -Ilib/librte_eal.a.p -Ilib -I../lib -I. -I.. -Iconfig
-I../config -Ilib/librte_eal/include -I../lib/librte_eal/include
-Ilib/librte_eal/linux/include -I../lib/librte_eal/linux/include
-Ilib/librte_eal/x86/include -I../lib/librte_eal/x86/include
-Ilib/librte_eal/common -I../lib/librte_eal/common -Ilib/librte_eal
-I../lib/librte_eal -Ilib/librte_kvargs -I../lib/librte_kvargs
-Ilib/librte_metrics -I../lib/librte_metrics -Ilib/librte_telemetry
-I../lib/librte_telemetry -pipe -D_FILE_OFFSET_BITS=64 -Wall
-Winvalid-pch -O3 -include rte_config.h -Wextra -Wcast-qual
-Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security
-Wmissing-declarations -Wmissing-prototypes -Wnested-externs
-Wold-style-definition -Wpointer-arith -Wsign-compare
-Wstrict-prototypes -Wundef -Wwrite-strings
-Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=native
-DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API '-DABI_VERSION="21.1"'
-MD -MQ lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o -MF
lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o.d -o
lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o -c
../lib/librte_eal/common/eal_common_config.c
In file included from ../lib/librte_eal/include/rte_dev.h:24:0,
                 from ../lib/librte_eal/common/eal_private.h:12,
                 from ../lib/librte_eal/common/eal_common_config.c:9:
../lib/librte_eal/include/rte_compat.h:22:51: error: missing binary
operator before token "("
 #if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
                                                   ^
../lib/librte_eal/include/rte_compat.h:28:53: error: missing binary
operator before token "("
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /*
For clang */
                                                     ^

I can see that gcc doc recommends checking for __has_attribute availability.
Pasting from google cache, since the gcc.gnu.org doc website seems unavailable.

"""
4.2.6 __has_attribute

The special operator __has_attribute (operand) may be used in ‘#if’
and ‘#elif’ expressions to test whether the attribute referenced by
its operand is recognized by GCC. Using the operator in other contexts
is not valid. In C code, if compiling for strict conformance to
standards before C2x, operand must be a valid identifier. Otherwise,
operand may be optionally introduced by the attribute-scope:: prefix.
The attribute-scope prefix identifies the “namespace” within which the
attribute is recognized. The scope of GCC attributes is ‘gnu’ or
‘__gnu__’. The __has_attribute operator by itself, without any operand
or parentheses, acts as a predefined macro so that support for it can
be tested in portable code. Thus, the recommended use of the operator
is as follows:

#if defined __has_attribute
#  if __has_attribute (nonnull)
#    define ATTR_NONNULL __attribute__ ((nonnull))
#  endif
#endif

The first ‘#if’ test succeeds only when the operator is supported by
the version of GCC (or another compiler) being used. Only when that
test succeeds is it valid to use __has_attribute as a preprocessor
operator. As a result, combining the two tests into a single
expression as shown below would only be valid with a compiler that
supports the operator but not with others that don’t.

#if defined __has_attribute && __has_attribute (nonnull)   /* not portable */
…
#endif
"""



-- 
David Marchand