> From: Stephen Hemminger [mailto:[email protected]]
> Sent: Wednesday, 2 October 2024 17.43
>
> The allocation functions take a alignment argument that
> can be useful to hint the compiler optimizer. This is supported
> by GCC and Clang but only useful with GCC because Clang gives
> warning if alignment is 0.
>
> Newer versions of GCC have a malloc attribute that can be used to find
> mismatches between allocation and free; the typical problem caught is a
> pointer allocated with rte_malloc() that is then incorrectly freed
> using
> free(). The name of the DPDK wrapper macros for these attributes
> are chosen to be similar to what GLIBC is using in cdefs.h.
> Note: The rte_free function prototype was moved ahead of the allocation
> functions since the dealloc attribute now refers to it.
>
> Signed-off-by: Stephen Hemminger <[email protected]>
> Acked-by: Chengwen Feng <[email protected]>
> Acked-by: Anatoly Burakov <[email protected]>
> ---
I see many of my comments to v3 have already been addressed. Great minds think
alike. :-)
> +/**
> + * With recent GCC versions also able to track that proper
> + * deallocator function is used for this pointer.
> + */
> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 110000)
> +#define __rte_dealloc(dealloc, argno) \
> + __attribute__((malloc(dealloc, argno)))
> +#else
> +#define __rte_dealloc(dealloc, argno)
> +#endif
A matter of taste...
The name "__rte_malloc" is closely associated with the function name
"malloc()"; for consistency suggest naming this "__rte_free" or
"__rte_malloc_free".
<brainstorming>
If named __rte_malloc_free, it could include the __rte_malloc (as in previous
versions of the patch).
However, that might be confusing, so probably not a good idea.
I prefer keeping the attributes separate, as in this version.
</brainstorming>
> +++ b/lib/eal/include/rte_malloc.h
> @@ -31,6 +31,26 @@ struct rte_malloc_socket_stats {
> size_t heap_allocsz_bytes; /**< Total allocated bytes on heap */
> };
>
> +/**
> + * Function attribut for prototypes that expect to release memory with
> rte_free()
Typo: attribut -> attribute
> + */
> +#define __rte_dealloc_free __rte_dealloc(rte_free, 1)
Minor detail:
I'm worried someone might misunderstand the purpose of this shortcut, and use
it with an allocator function where a different deallocator is associated.
Moving it from rte_common.h to rte_malloc.h is a huge improvement; but please
consider if the benefit outweighs the risk.
Again, I'll leave it up to you.
