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 1F164428C6; Tue, 4 Apr 2023 18:28:56 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C19041156; Tue, 4 Apr 2023 18:28:56 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 3111741611 for ; Tue, 4 Apr 2023 18:28:54 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 5A2A8210DD99; Tue, 4 Apr 2023 09:28:53 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 5A2A8210DD99 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1680625733; bh=CWEAlmQ/LqLFugCR6+aawzejVnhQUjp5RVNCUzfsHqU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jj6R5SWvPGTwalCt30kl8ecpX7o96Qrtu/dE+Bqw8vfF+QOkfo0ojhtOkddn3cVfk 90NDM0iMW2lTEpLegYDTka4WMvTHnEtfPbzqhqUoERDsmp2AX19QHOBwmkwqq/Db5Y hT3A5SHi5VTC5A78M805Iawu8MhdxQo6jNm6evBI= Date: Tue, 4 Apr 2023 09:28:53 -0700 From: Tyler Retzlaff To: Bruce Richardson Cc: Konstantin Ananyev , "dev@dpdk.org" , "ciara.power@intel.com" , "david.marchand@redhat.com" , "thomas@monjalon.net" Subject: Re: [PATCH v3] telemetry: use portable syntax to initialize array Message-ID: <20230404162853.GC18560@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1680539424-20255-1-git-send-email-roretzla@linux.microsoft.com> <1680548365-16525-1-git-send-email-roretzla@linux.microsoft.com> <1680548365-16525-2-git-send-email-roretzla@linux.microsoft.com> <65480e4a272445779710dcc4c6f21dfa@huawei.com> <20230404155906.GE23247@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 Tue, Apr 04, 2023 at 05:19:26PM +0100, Bruce Richardson wrote: > On Tue, Apr 04, 2023 at 08:59:06AM -0700, Tyler Retzlaff wrote: > > On Tue, Apr 04, 2023 at 09:01:50AM +0000, Konstantin Ananyev wrote: > > > > > > > > > > -----Original Message----- > > > > From: Tyler Retzlaff > > > > Sent: Monday, April 3, 2023 7:59 PM > > > > To: dev@dpdk.org > > > > Cc: ciara.power@intel.com; bruce.richardson@intel.com; david.marchand@redhat.com; thomas@monjalon.net; Tyler Retzlaff > > > > > > > > Subject: [PATCH v3] telemetry: use portable syntax to initialize array > > > > > > > > Use of ranges in designated initialization are a non-standard gcc > > > > extension. Use loops to initialize permitted characters on first use. > > > > > > > > Signed-off-by: Tyler Retzlaff > > > > --- > > > > lib/telemetry/telemetry_data.c | 22 +++++++++++++++------- > > > > 1 file changed, 15 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c > > > > index 2bac2de..562b387 100644 > > > > --- a/lib/telemetry/telemetry_data.c > > > > +++ b/lib/telemetry/telemetry_data.c > > > > @@ -152,13 +152,21 @@ > > > > static bool > > > > valid_name(const char *name) > > > > { > > > > - char allowed[128] = { > > > > - ['0' ... '9'] = 1, > > > > - ['A' ... 'Z'] = 1, > > > > - ['a' ... 'z'] = 1, > > > > - ['_'] = 1, > > > > - ['/'] = 1, > > > > - }; > > > > + int index; > > > > + static bool initialized; > > > > + static char allowed[128]; > > > > + > > > > + if (!initialized) { > > > > + for (index = '0'; index <= '9'; index++) > > > > + allowed[index] = 1; > > > > + for (index = 'A'; index <= 'Z'; index++) > > > > + allowed[index] = 1; > > > > + for (index = 'a'; index <= 'z'; index++) > > > > + allowed[index] = 1; > > > > + allowed[(int)'_'] = allowed[(int)'/'] = 1; > > > > + initialized = true; > > > > + } > > > > + > > > > while (*name != '\0') { > > > > if ((size_t)*name >= RTE_DIM(allowed) || allowed[(int)*name] == 0) > > > > return false; > > > > > > Wonder isn't the same as: > > > while (*name != 0) > > > if (!isascii(name[0] || (!isalnum(name[0]) && name[0] != '_' && name[0] != '/')) > > > return false; > > > > > > Or MSVC doesn't support these macros? > > > > it's standard C msvc supports it. Bruce acceptable to take Konstantin's > > suggestion here? > > > Yep, I'm ok with it. > > If I may confuse things a little, how about combining the two? Use an array > with individual initializers for the "special characters", but using > isalnum() for the rest. That way we get a clear list of allowed chars at > the top of the array, without having a massive list of individual letters > and numbers. For example: [completely untested and not compiled!]. > > { > /* non-alpha-numeric characters allowed in names */ > static const char allowed[128] = { ['_'] = 1, ['/'] = 1 }; > > for ( ; *name != '\0'; name++) { > if (isalnum(*name)) > continue; > if ((size_t)*name >= RTE_DIM(allowed) || allowed[(int)*name] == 0) > return false; > } > return true; > } > > I think something like this achieves a good balance between clarity and > brevity. > 1. We don't have a massive array definition > 2. We do have a static constant array > 3. We don't have the non-alnum character list hidden inside the middle of > an "if" statement. at first glance i'm not sure if that initializer list syntax is portable but i guess i'll find out. i'm fine with being confused more so long as you give me the answer and i can just cut and paste it ;) > > /Bruce