From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ananyev, Konstantin" Subject: Re: [PATCH v4 6/9] app/pdump: add pdump tool for packet capturing Date: Thu, 2 Jun 2016 14:22:27 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B6ACB1@irsmsx105.ger.corp.intel.com> References: <1463503030-10318-1-git-send-email-reshma.pattan@intel.com> <1464039512-2683-1-git-send-email-reshma.pattan@intel.com> <1464039512-2683-7-git-send-email-reshma.pattan@intel.com> <2601191342CEEE43887BDE71AB97725836B6810D@irsmsx105.ger.corp.intel.com> <3AEA2BF9852C6F48A459DA490692831F01045487@IRSMSX109.ger.corp.intel.com> <2601191342CEEE43887BDE71AB97725836B692DD@irsmsx105.ger.corp.intel.com> <3AEA2BF9852C6F48A459DA490692831F01046BA3@IRSMSX109.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable To: "Pattan, Reshma" , "dev@dpdk.org" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 7322558CF for ; Thu, 2 Jun 2016 16:22:35 +0200 (CEST) In-Reply-To: <3AEA2BF9852C6F48A459DA490692831F01046BA3@IRSMSX109.ger.corp.intel.com> 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" > -----Original Message----- > From: Pattan, Reshma > Sent: Thursday, June 02, 2016 1:32 PM > To: Ananyev, Konstantin; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v4 6/9] app/pdump: add pdump tool for pack= et capturing >=20 >=20 >=20 > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Tuesday, May 31, 2016 6:21 PM > > To: Pattan, Reshma ; dev@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v4 6/9] app/pdump: add pdump tool for pa= cket > > capturing > > > > > > > > > -----Original Message----- > > > From: Pattan, Reshma > > > Sent: Tuesday, May 31, 2016 3:50 PM > > > To: Ananyev, Konstantin; dev@dpdk.org > > > Subject: RE: [dpdk-dev] [PATCH v4 6/9] app/pdump: add pdump tool for > > > packet capturing > > > > > > > > > > > > > -----Original Message----- > > > > From: Ananyev, Konstantin > > > > Sent: Friday, May 27, 2016 4:22 PM > > > > To: Pattan, Reshma ; dev@dpdk.org > > > > Cc: Pattan, Reshma > > > > Subject: RE: [dpdk-dev] [PATCH v4 6/9] app/pdump: add pdump tool fo= r > > > > packet capturing > > > > > > > > > > > > > +static int > > > > > +parse_num_mbufs(const char *key __rte_unused, const char *value, > > > > > +void > > > > > +*extra_args) { > > > > > + int n; > > > > > + struct pdump_tuples *pt =3D extra_args; > > > > > + > > > > > + n =3D atoi(value); > > > > > + if (n > 1024) > > > > > + pt->total_num_mbufs =3D (uint16_t) n; > > > > > + else { > > > > > + printf("total-num-mbufs %d invalid - must be > 1024\n", n); > > > > > + return -1; > > > > > + } > > > > > + > > > > > + return 0; > > > > > +} > > > > > > > > > > > > You have several parse functions - doing almost the same thing: > > > > convert string to integer value and then check that this valu is > > > > within specific range. > > > > Why not to introduce one function that would accept as extra_args > > > > pointer to the struct {uint64_t v; uint64_t min; uint64_t max; } So > > > > inside that function you can check that: v >=3D min && v < max or s= o. > > > > Then you can use that function all over the places. > > > > Another possibility just have parse function that only does > > > > conversion without any boundary checking, and make boundary check l= ater > > in parse_pdump(). > > > > In both cases you can re-use same parse function. > > > > > > > > > > Yes, I do have 4 functions but in all I am not checking min and max > > > values and log message also differs in each function. So I would lik= e to retain > > this as it is now. > > > > What for? > > >=20 > OK, I will make changes to have parse function only for conversion and b= oundary checks will be done > In parse_pdump(). This looks ok to me. >=20 > I agree with rest of the comments and will take care of the same in next = revision of patch set. Ok, thanks. Konstantin