Re: [dpdk-dev] [PATCH v2 1/7] eal: add wrappers for POSIX string functions
From: Thomas Monjalon <hidden>
Date: 2021-03-16 09:51:25
27/02/2021 21:23, Dmitry Kozlyuk:
2021-02-23 09:45, Bruce Richardson:quoted
On Tue, Feb 23, 2021 at 01:57:50AM +0300, Dmitry Kozlyuk wrote:quoted
2021-02-22 14:26, Bruce Richardson:quoted
As you say, though, the main issue will be whether we have instances in public header files or not. I would hope that no static inline functions in DPDK use any of the functions in question, but I'm not sure. Perhaps if there are instances in public headers those could be reworked to not use the problematic functions.No instances of strdup(), strncasecmp(), or strtok_r() in any DPDK headers.quoted
For any functions, such as strdup, which are not in a public header I would suggest the following as a possible start point, based off what was done for strlcpy. * In DPDK (probably EAL), define an rte_strdup function for use as a fallback. * Inside the meson build scripts, use "cc.has_function()" to check if the regular strdup function is available. If not, then add "-DRTE_NO_STRDUP" to the c_args for DPDK building * Inside our DPDK header (rte_string_fns.h in the strdup case), we can add a conditional define such as: #ifdef RTE_NO_STRDUP #define strdup(s) rte_strdup(s) #endif Thoughts on this?Looks good to me, I can rework the patchset like so. Policy considerations: 1. The approach only applies to platform-agnostic functions, like str*(). Functions like sleep() still belong to librte_eal. 2. Deprecated functions, like index(3p), should be replaced with alternatives suggested by the standard. 3. If a standard C11 alternative is available, it should be used. This mostly applies to types, like u_int32 -> uint32_t (it's even in DPDK coding style already, isn't it?). A nit: RTE_NO_XXX -> RTE_HAS_XXX (for consistency with existing macros)?Sure, thanks.There's a meson issue with `cc.has_function()`: https://github.com/mesonbuild/meson/issues/5628
The meson issue can be fixed or workarounded probably. Is it the reason for the RTE_INTERNAL proposal below?
What if we just define RTE_INTERNAL for librte_eal/windows/include/rte_os.h (and other public headers if need be) to distinguish the case when it's used from within DPDK?
I'm not sure to follow the need for RTE_INTERNAL. In general, 3 guidelines: - avoid inline functions in public headers - mark exported internal functions with __rte_internal and in version.map - export internal functions in a separate file