From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Dai, Wei" Subject: Re: [PATCH v2 1/2] eal: remove redundant codes to parse --lcores Date: Wed, 27 Jul 2016 09:15:27 +0000 Message-ID: <49759EB36A64CF4892C1AFEC9231E8D63A266397@PGSMSX106.gar.corp.intel.com> References: <1469081018-55300-1-git-send-email-wei.dai@intel.com> <1469526749-19698-1-git-send-email-wei.dai@intel.com> <2601191342CEEE43887BDE71AB97725836B820CA@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: Adam Bynes , "Ananyev, Konstantin" Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 1329E530C for ; Wed, 27 Jul 2016 11:15:30 +0200 (CEST) In-Reply-To: Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, Adam & Ananyev Thanks for your feedback. > -----Original Message----- > From: Adam Bynes [mailto:adambynes@outlook.com] > Sent: Wednesday, July 27, 2016 1:36 AM > To: Dai, Wei ; Ananyev, Konstantin > > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/2] eal: remove redundant codes to par= se > --lcores >=20 > On Tue, Jul 26, 2016 at 11:51:57AM +0000, Ananyev, Konstantin wrote: > > > > > hi Wei, > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai > > > Sent: Tuesday, July 26, 2016 10:52 AM > > > To: dev@dpdk.org > > > Cc: Dai, Wei > > > Subject: [dpdk-dev] [PATCH v2 1/2] eal: remove redundant codes to par= se > --lcores > > > > > > local variable i is not referred by other codes in the function > eal_parse_lcores( ), so it can be removed. > > > > > > Signed-off-by: Wei Dai > > > --- > > > lib/librte_eal/common/eal_common_options.c | 4 ---- > > > 1 file changed, 4 deletions(-) > > > > > > diff --git a/lib/librte_eal/common/eal_common_options.c > b/lib/librte_eal/common/eal_common_options.c > > > index 481c732..c5bf98c 100644 > > > --- a/lib/librte_eal/common/eal_common_options.c > > > +++ b/lib/librte_eal/common/eal_common_options.c > > > @@ -578,7 +578,6 @@ eal_parse_lcores(const char *lcores) > > > struct rte_config *cfg =3D rte_eal_get_configuration(); > > > static uint16_t set[RTE_MAX_LCORE]; > > > unsigned idx =3D 0; > > > - int i; > > > unsigned count =3D 0; > > > const char *lcore_start =3D NULL; > > > const char *end =3D NULL; > > > @@ -593,9 +592,6 @@ eal_parse_lcores(const char *lcores) > > > /* Remove all blank characters ahead and after */ > > > while (isblank(*lcores)) > > > lcores++; > > > - i =3D strlen(lcores); > > > - while ((i > 0) && isblank(lcores[i - 1])) > > > - i--; > > > > I suppose originally it meant to do something like that: > > while ((i > 0) && isblank(lcores[i - 1])) > > lcores[i--] =3D 0; > totally agreed Konstantin, need to add lcore[i--] =3D '\0' >=20 > > > > to get rid of blank characters at the end of the line, no? > > Konstantin The tail blank is not necessary to be removed from lcores here for followin= g reasons: 1. The tail blanks can also be swallowed later in function eal_parse_set( )= =20 by "while (isblank(*end)) end++". And such operation/sentence also deal with blanks in the middle of arguments (for example: blank before - and after (7,8) in '0 -2 ,(3 - 6)@(3-6),7@(7-8) ,8@(8-10) = '),=20 so above removal of blank is redundant. By the way, with --lcores '(0-3)@(0-3), (4-5)@(4-5) ', I also find a n= ew bug. After processing cpu set (4-5), the variable end in eal_parse_lcores does= n't point t ',' or '\0', so this function return an error. The tail blank after cpu set (4-5) still need to be swallowed.=20 The patch v3 will be removed.eal 2. if let lcores[i--] =3D 0 here, due to type of input argument lcores (con= st char *),=20 building will fail. And if the type is changed to char *, the type of inp= ut argument of several other function also need to be changed. So according to above rea= son 1, it is not need to change the type. Thanks Wei Dai=20 > > > > > > > > CPU_ZERO(&cpuset); > > > > > > -- > > > 2.5.5 > Adam Bynes