From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2CD1EA00BE; Mon, 14 Mar 2022 10:00:30 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BDA2940DF4; Mon, 14 Mar 2022 10:00:29 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mails.dpdk.org (Postfix) with ESMTP id D810440DDD for ; Mon, 14 Mar 2022 10:00:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1647248428; x=1678784428; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=OjzC4DBvv0daZGgYoP6HfIcnRp8JOALdUXg5e0BniEk=; b=Qxvvz2kXATCY3W4KxHkfAKdOz9cXtGyD5YBRy/8IReU6F+dMtOgWFCyk uYAar6x7ZsznUnfAQZ7iXc822NkXkh6KffwafkoLgmPwnh9EODKeJkcYV NdR/M8UEnvMVaFmfYXqSrAzPkXJGA2X3fhjqKWzoqNXygm6ZeHRNYvcGw 9PPXUSPxOpwqqKZ/itG1Wy/hy2kLuKMUz0dbg/Kw6VHFdYtH81H7ilUWY q/l1x6crNGePcalXKpgDvvswgFfgN4OZFL9s9uXkNRtjnNIzThHv9qvF8 FvlN3TbsIoYrCi/x1G2LpNmNXgOZPyLVF0T/TIyWSRkhP19kx+YUIhIzU Q==; X-IronPort-AV: E=McAfee;i="6200,9189,10285"; a="280732386" X-IronPort-AV: E=Sophos;i="5.90,180,1643702400"; d="scan'208";a="280732386" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Mar 2022 02:00:24 -0700 X-IronPort-AV: E=Sophos;i="5.90,180,1643702400"; d="scan'208";a="539882061" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.30.23]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 14 Mar 2022 02:00:23 -0700 Date: Mon, 14 Mar 2022 09:00:20 +0000 From: Bruce Richardson To: Stephen Hemminger Cc: dev@dpdk.org, pbhagavatula@marvell.com Subject: Re: [PATCH 5/5] eventdev: fix compilation with clang C++ builds Message-ID: References: <20220311200523.1020050-1-bruce.richardson@intel.com> <20220311200523.1020050-6-bruce.richardson@intel.com> <20220311171102.70e3d1f9@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220311171102.70e3d1f9@hermes.local> X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Fri, Mar 11, 2022 at 05:11:02PM -0800, Stephen Hemminger wrote: > On Fri, 11 Mar 2022 20:05:23 +0000 > Bruce Richardson wrote: > > > When compiling on FreeBSD with clang and include checking enabled, > > errors are emitted due to differences in how empty structs/unions are > > handled in C and C++, as C++ structs cannot have zero size. > > > > ../lib/eventdev/rte_eventdev.h:992:2: error: union has size 0 in C, non-zero size in C++ > > > > Since the contents of the union are all themselves of zero size, > > the actual union wrapper is unnecessary. We therefore remove it for C++ > > builds - though keep it for C builds for safety and clarity of > > understanding the code. The alignment constraint on the union is > > unnecessary in the case where the whole struct is aligned on a 16-byte > > boundary, so we add that constraint to the overall structure to ensure > > it applies for C++ code as well as C. > > > > Fixes: 1cc44d409271 ("eventdev: introduce event vector capability") > > Cc: pbhagavatula@marvell.com > > > > Signed-off-by: Bruce Richardson > > --- > > lib/eventdev/rte_eventdev.h | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h > > index 67c4a5e036..42a5660169 100644 > > --- a/lib/eventdev/rte_eventdev.h > > +++ b/lib/eventdev/rte_eventdev.h > > @@ -984,21 +984,31 @@ struct rte_event_vector { > > }; > > /**< Union to hold common attributes of the vector array. */ > > uint64_t impl_opaque; > > + > > +/* empty structures do not have zero size in C++ leading to compilation errors > > + * with clang about structure having different sizes in C and C++. > > + * Since these are all zero-sized arrays, we can omit the "union" wrapper for > > + * C++ builds, removing the warning. > > + */ > > +#ifndef __cplusplus > > /**< Implementation specific opaque value. > > * An implementation may use this field to hold implementation specific > > * value to share between dequeue and enqueue operation. > > * The application should not modify this field. > > */ > > union { > > +#endif > > struct rte_mbuf *mbufs[0]; > > void *ptrs[0]; > > uint64_t *u64s[0]; > > +#ifndef __cplusplus > > } __rte_aligned(16); > > +#endif > > /**< Start of the vector array union. Depending upon the event type the > > * vector array can be an array of mbufs or pointers or opaque u64 > > * values. > > */ > > -}; > > +} __rte_aligned(16); > > > > /* Scheduler type definitions */ > > #define RTE_SCHED_TYPE_ORDERED 0 > > Zero size arrays should be replaced by flexible arrays. > Linux has this coccinelle script for that: > Yes, I agree. However, given how late in the release process I discovered this, I wanted a fix with absolute minimum impact, which is why I chose the above. Zero struct change for C code, and enough of a fix for C++ to get it compiling. If you feel that replacing with flexible arrays is safe enough to do at this late stage, we can perhaps consider it, but overall I'd suggest doing any such replacement in 22.07 /Bruce