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 965454331D; Mon, 13 Nov 2023 18:57:42 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 88B6540E36; Mon, 13 Nov 2023 18:57:42 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 865084026F for ; Mon, 13 Nov 2023 18:57:40 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id AF1DA20B74C1; Mon, 13 Nov 2023 09:57:39 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AF1DA20B74C1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1699898259; bh=dciOUG1KzhGqLoXXBbYWz45p6TRxPqKy+5pPB/G2rRs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Skpa2TfqnpLnQEMksnsFzvlvQuK8kiDC7l6L6pzIUdmMs6mnY6J2Bhxmb8RcvS5Dc LutXgjECAAn13e3tIBralH/gOSqnSLWhrX7deXevGN+cR3Z8dYlb6UiTcQ0f2Q2Y5z ahu8kqW/IK9ag14RqNYqsXG3tNRex6b4KMES88wE= Date: Mon, 13 Nov 2023 09:57:39 -0800 From: Tyler Retzlaff To: Bruce Richardson Cc: Stephen Hemminger , dev@dpdk.org, Morten =?iso-8859-1?Q?Br=F8rup?= Subject: Re: [PATCH v2 3/3] eal: replace out of bounds VLA with static_assert Message-ID: <20231113175739.GC32289@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20231111172153.57461-1-stephen@networkplumber.org> <20231113170605.408281-1-stephen@networkplumber.org> <20231113170605.408281-4-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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 Mon, Nov 13, 2023 at 05:12:48PM +0000, Bruce Richardson wrote: > On Mon, Nov 13, 2023 at 09:06:05AM -0800, Stephen Hemminger wrote: > > Both Gcc, clang and MSVC have better way to do compile time > > assertions rather than using out of bounds array access. > > The old method would fail if -Wvla is enabled because compiler > > can't determine size in that code. Also, the use of new > > _Static_assert will catch broken code that is passing non-constant > > expression to RTE_BUILD_BUG_ON(). > > > > Signed-off-by: Stephen Hemminger > > Acked-by: Morten Brørup > > Acked-by: Tyler Retzlaff > > --- > > lib/eal/include/rte_common.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h > > index c1ba32d00e47..bea7c0e57d5e 100644 > > --- a/lib/eal/include/rte_common.h > > +++ b/lib/eal/include/rte_common.h > > @@ -16,6 +16,7 @@ > > extern "C" { > > #endif > > > > +#include > > #include > > #include > > > > @@ -495,7 +496,7 @@ rte_is_aligned(const void * const __rte_restrict ptr, const unsigned int align) > > /** > > * Triggers an error at compilation time if the condition is true. > > */ > > -#define RTE_BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)])) > > +#define RTE_BUILD_BUG_ON(condition) static_assert(!(condition), #condition) > > > Acked-by: Bruce Richardson > > And as for the static_assert vs BUILD_BUG_ON debate, I think I'd go with > using static_assert explicitly and only keeping BUILD_BUG_ON for backward > compatbility. Reason being that you can give a reasonable error message > with the assert. my vote is for the same, i would prefer to just use standard constructs directly. i do wish the C11 version didn't require a message but regardless let's use the standard version. additionally, can we have an addition to checkpatches.pl (if the above is agreed) that prevents further additions of RTE_BUILD_BUG_ON usage?