On 3/9/2020 9:45 AM, Gavin Hu wrote: > Hi Ferruh, > >> -----Original Message----- >> From: Ferruh Yigit <ferruh.yi...@intel.com> >> Sent: Monday, March 9, 2020 4:55 PM >> To: Gavin Hu <gavin...@arm.com>; dev@dpdk.org >> Cc: nd <n...@arm.com>; david.march...@redhat.com; tho...@monjalon.net; >> ktray...@redhat.com; jer...@marvell.com; Honnappa Nagarahalli >> <honnappa.nagaraha...@arm.com>; Ruifeng Wang >> <ruifeng.w...@arm.com>; Phil Yang <phil.y...@arm.com>; Joyce Kong >> <joyce.k...@arm.com>; sta...@dpdk.org; Olivier MATZ >> <olivier.m...@6wind.com>; Konstantin Ananyev >> <konstantin.anan...@intel.com>; Andrew Rybchenko >> <arybche...@solarflare.com> >> Subject: Re: [dpdk-dev] [PATCH v2] mbuf: replace zero-length marker with >> unnamed union >> >> On 3/7/2020 3:56 PM, Gavin Hu wrote: >>> Declaring zero-length arrays in other contexts, including as interior >>> members of structure objects or as non-member objects, is discouraged. >>> Accessing elements of zero-length arrays declared in such contexts is >>> undefined and may be diagnosed.[1] >>> >>> Fix by using unnamed union and struct. >>> >>> https://bugs.dpdk.org/show_bug.cgi?id=396 >>> >>> Bugzilla ID: 396 >>> >>> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html >>> >>> Fixes: 3e6181b07038 ("mbuf: use structure marker from EAL") >>> Cc: sta...@dpdk.org >>> >>> Signed-off-by: Gavin Hu <gavin...@arm.com> >>> --- >>> v2: >>> * change 'uint64_t rearm_data' to 'uint_64_t rearm_data[1]' to fix >>> the SFC PMD compiling error on x86. <Kevin Traynor> >>> --- >>> lib/librte_mbuf/rte_mbuf_core.h | 54 +++++++++++++++++++-------------- >>> 1 file changed, 32 insertions(+), 22 deletions(-) >>> >>> diff --git a/lib/librte_mbuf/rte_mbuf_core.h >> b/lib/librte_mbuf/rte_mbuf_core.h >>> index b9a59c879..34cb152e2 100644 >>> --- a/lib/librte_mbuf/rte_mbuf_core.h >>> +++ b/lib/librte_mbuf/rte_mbuf_core.h >>> @@ -480,31 +480,41 @@ struct rte_mbuf { >>> rte_iova_t buf_physaddr; /**< deprecated */ >>> } __rte_aligned(sizeof(rte_iova_t)); >>> >>> - /* next 8 bytes are initialised on RX descriptor rearm */ >>> - RTE_MARKER64 rearm_data; >>> - uint16_t data_off; >>> - >>> - /** >>> - * Reference counter. Its size should at least equal to the size >>> - * of port field (16 bits), to support zero-copy broadcast. >>> - * It should only be accessed using the following functions: >>> - * rte_mbuf_refcnt_update(), rte_mbuf_refcnt_read(), and >>> - * rte_mbuf_refcnt_set(). The functionality of these functions (atomic, >>> - * or non-atomic) is controlled by the >> CONFIG_RTE_MBUF_REFCNT_ATOMIC >>> - * config option. >>> - */ >>> RTE_STD_C11 >>> union { >>> - rte_atomic16_t refcnt_atomic; /**< Atomically accessed >> refcnt */ >>> - /** Non-atomically accessed refcnt */ >>> - uint16_t refcnt; >>> - }; >>> - uint16_t nb_segs; /**< Number of segments. */ >>> + /* next 8 bytes are initialised on RX descriptor rearm */ >>> + uint64_t rearm_data[1]; >> We are using zero length array as markers only and know what we are doing >> with them, >> what would you think disabling the warning instead of increasing the >> complexity >> in mbuf struct? > Okay, I will add -Wno-zero-length-bounds to the compiler toolchain flags.
This would be my preference but I would like to get more input, can you please for more comments before changing the implementation in case there are some strong opinion on it?
