From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 02 Jun 2016 07:22:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,406,1459839600"; d="scan'208";a="979506524" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by fmsmga001.fm.intel.com with ESMTP; 02 Jun 2016 07:22:30 -0700 Received: from irsmsx112.ger.corp.intel.com (10.108.20.5) by IRSMSX106.ger.corp.intel.com (163.33.3.31) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 2 Jun 2016 15:22:28 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.51]) by irsmsx112.ger.corp.intel.com ([10.108.20.5]) with mapi id 14.03.0248.002; Thu, 2 Jun 2016 15:22:28 +0100 From: "Ananyev, Konstantin" To: "Pattan, Reshma" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v4 6/9] app/pdump: add pdump tool for packet capturing Thread-Index: AQHRtTxSrttfe/yqk0ujIi7IDWMW7Z/M27aAgAZAgoCAADrj4IACwxgAgAAvn9A= 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> In-Reply-To: <3AEA2BF9852C6F48A459DA490692831F01046BA3@IRSMSX109.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 6/9] app/pdump: add pdump tool for packet capturing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jun 2016 14:22:36 -0000 > -----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