Re: [PATCH v5 1/4] eal: add nonnull and access function attributes

[David Marchand <david.marchand@redhat.com>]

On Wed, Dec 28, 2022 at 4:10 PM Morten Brørup <mb@smartsharesystems.com> wrote:
>
> Add "nonnull" function attribute to help the compiler detect a NULL
> pointer being passed to a function not accepting NULL pointers as an
> argument at build time.
>
> Add "access" function attribute to tell the compiler how a function
> accesses its pointer arguments.
>
> Add these attributes to the rte_memcpy() function, as the first in
> hopefully many to come.
>

Compilation is broken starting first patch, so patches must be
reordered to have the fixes first.


> v5:
> * No changes.
> v4:
> * No changes.
> v3:
> * No changes.
> v2:
> * Only define "nonnull" for GCC and CLANG.
> * Append _param/_params to prepare for possible future attributes
>   attached directly to the individual parameters, like __rte_unused.
> * Use RTE_TOOLCHAIN_GCC instead of RTE_CC_GCC, to fix complaints about
>   GCC_VERSION being undefined.
> * Try to fix Doxygen compliants.

Patch history should be put as annotations (i.e. after --- but before
patch content).


>
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

[snip]


> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> index 15765b408d..6e4011aa85 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -149,6 +149,35 @@ typedef uint16_t unaligned_uint16_t;
>         __attribute__((format(printf, format_index, first_arg)))
>  #endif
>
> +/**
> + * Check pointer arguments at compile-time.
> + *
> + * @param ...
> + *    Comma separated list of parameter indexes of pointer arguments.
> + */
> +#if defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
> +#define __rte_nonnull_params(...) \
> +       __attribute__((nonnull(__VA_ARGS__)))
> +#else
> +#define __rte_nonnull_params(...)
> +#endif
> +
> +/**
> + * Tells compiler about the access mode of a pointer argument.
> + *
> + * @param access_mode
> + *    Access mode: read_only, read_write, write_only, or none.
> + * @param ...
> + *    Parameter index of pointer argument.
> + *    Optionally followeded by comma and parameter index of size argument.
> + */
> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 100400)
> +#define __rte_access_param(access_mode, ...) \
> +       __attribute__((access(access_mode, __VA_ARGS__)))
> +#else
> +#define __rte_access_param(access_mode, ...)
> +#endif
> +

This is tightly bound to gcc syntax.
With dedicated macros (which I find easier to read too), we can hope
to adapt to other compilers if some of them add support for this kind
of code cookies.
__rte_read_only_params(indexes...)
__rte_write_only_params(indexes...)
__rte_no_access_params(indexes...)


-- 
David Marchand