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 A872643B19; Thu, 15 Feb 2024 18:32:10 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 71F17402AD; Thu, 15 Feb 2024 18:32:10 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 458E640276 for ; Thu, 15 Feb 2024 18:32:08 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 6D856207F21B; Thu, 15 Feb 2024 09:32:07 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 6D856207F21B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1708018327; bh=nlWG+Xv2FCRckrMAzXM3ZFSg2fWtDppIY8anuMFiCAg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=VUT0RO7zJDD6M/VIVFtF17edEUD28gw4gx2Mq9CF3upbTpL94rOzRlBp0ODIZjBhE K4r7Au4MQKfVXLPQpfADCi0DFan9BPoiEAxByRgCrlEWeCkYn78ykT1oWH3FRvHdgc nlDJYUVX+AzmBvr2yM5oSeIPhqxBnoertOY7MGwk= Date: Thu, 15 Feb 2024 09:32:07 -0800 From: Tyler Retzlaff To: Bruce Richardson Cc: David Marchand , dev@dpdk.org, Sunil Kumar Kori , Rakesh Kudurumalla , Jerin Jacob , Srikanth Yalavarthi , Cristian Dumitrescu , Aman Singh , Yuying Zhang , Brian Dooley , Gowrishankar Muthukrishnan Subject: Re: [PATCH] eal: add helper to skip whitespaces Message-ID: <20240215173207.GA556@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20240214121219.3408867-1-david.marchand@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 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 Thu, Feb 15, 2024 at 09:10:58AM +0000, Bruce Richardson wrote: > On Thu, Feb 15, 2024 at 10:08:40AM +0100, David Marchand wrote: > > On Wed, Feb 14, 2024 at 2:07 PM Bruce Richardson > > wrote: > > > > > > On Wed, Feb 14, 2024 at 01:12:19PM +0100, David Marchand wrote: > > > > Reduce code duplication by providing a simple inline helper. > > > > > > > > Signed-off-by: David Marchand > > > > --- > > > > app/graph/utils.c | 13 ++------ > > > > app/test-eventdev/parser.c | 14 ++++----- > > > > app/test-eventdev/parser.h | 8 ----- > > > > app/test-mldev/parser.c | 17 ++++++----- > > > > app/test-mldev/parser.h | 8 ----- > > > > app/test-pmd/cmdline_tm.c | 13 ++------ > > > > app/test/test_string_fns.c | 35 ++++++++++++++++++++++ > > > > examples/fips_validation/fips_validation.c | 16 +++------- > > > > examples/ip_pipeline/parser.c | 14 ++++----- > > > > examples/ip_pipeline/parser.h | 8 ----- > > > > examples/pipeline/cli.c | 13 ++------ > > > > lib/eal/include/rte_string_fns.h | 27 +++++++++++++++++ > > > > 12 files changed, 98 insertions(+), 88 deletions(-) > > > > > > > > diff --git a/app/graph/utils.c b/app/graph/utils.c > > > > index c7b6ae83cf..1f5ee68273 100644 > > > > --- a/app/graph/utils.c > > > > +++ b/app/graph/utils.c > > > > @@ -9,17 +9,10 @@ > > > > #include > > > > > > > > #include > > > > +#include > > > > > > > > #include "module_api.h" > > > > > > > > -#define white_spaces_skip(pos) \ > > > > -({ \ > > > > - __typeof__(pos) _p = (pos); \ > > > > - for ( ; isspace(*_p); _p++) \ > > > > - ; \ > > > > - _p; \ > > > > -}) > > > > - > > > > static void > > > > hex_string_to_uint64(uint64_t *dst, const char *hexs) > > > > { > > > > @@ -47,7 +40,7 @@ parser_uint64_read(uint64_t *value, const char *p) > > > > char *next; > > > > uint64_t val; > > > > > > > > - p = white_spaces_skip(p); > > > > + p = rte_str_skip_whitespaces(p); > > > > if (!isdigit(*p)) > > > > return -EINVAL; > > > > > > > > @@ -73,7 +66,7 @@ parser_uint64_read(uint64_t *value, const char *p) > > > > break; > > > > } > > > > > > > > - p = white_spaces_skip(p); > > > > + p = rte_str_skip_whitespaces(p); > > > > > > I like the idea of this. However, a question on naming. Do we want to make > > > it clear in the name that it only skips leading whitespace, and doesn't > > > e.g. trim whitespace off the end? How about: > > > > > > rte_str_skip_leading_spaces() > > > > > > It's longer, but I wonder if it's a bit clearer. > > > > It is clearer. > > > > > > > > > > Also, might it be worthwhile providing an "rte_str_ltrim()" function along > > > with this, basically combining the skip_spaces call, with an memmove? Most > > > string handling in DPDK is not perf-sensitive and so a function with > > > similar name and behaviour to the python ltrim function may be appreciated > > > for its simplicity. > > > > Well, most intree users won't touch the data (and simply pass around > > const char * pointers). > > This new API you propose is probably fine, but I suspect it won't be > > of much use for now. > > > > Ok, good point. I'd suggest otherwise just going with your patch, perhaps > with the tweaked name. > > /Bruce whatever name is chosen I appreciate the patch. Also, this patch saves me from doing the same or something else later because all the duplicated macros exapnd to statement expressions. Acked-by: Tyler Retzlaff