From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 5323858CF for ; Thu, 2 Jun 2016 14:31:46 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga101.fm.intel.com with ESMTP; 02 Jun 2016 05:31:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,406,1459839600"; d="scan'208";a="967424393" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by orsmga001.jf.intel.com with ESMTP; 02 Jun 2016 05:31:44 -0700 Received: from irsmsx109.ger.corp.intel.com ([169.254.13.193]) by IRSMSX104.ger.corp.intel.com ([163.33.3.159]) with mapi id 14.03.0248.002; Thu, 2 Jun 2016 13:31:42 +0100 From: "Pattan, Reshma" To: "Ananyev, Konstantin" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v4 6/9] app/pdump: add pdump tool for packet capturing Thread-Index: AQHRuCt/TPztRZeOMEGHgoZxdsWGfJ/TJf9wgAAahwCAAuOFAA== Date: Thu, 2 Jun 2016 12:31:42 +0000 Message-ID: <3AEA2BF9852C6F48A459DA490692831F01046BA3@IRSMSX109.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> In-Reply-To: <2601191342CEEE43887BDE71AB97725836B692DD@irsmsx105.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTc1Y2JjZDktYzJmNi00MWU5LTg5MDUtZWFmZmE2MzJkZGU2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IktncmhTdjFLWVBHTVM5VElOQlNlUDBWekJQcTBkTXJRU3N2SUhlcFhBSEE9In0= x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.180] 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 12:31:46 -0000 > -----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 pack= et > capturing >=20 >=20 >=20 > > -----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 for > > > 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 so. > > > 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 lat= er > 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 like = to retain > this as it is now. >=20 > What for? >=20 OK, I will make changes to have parse function only for conversion and bou= ndary 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 re= vision of patch set. > > > > Thanks, > > Reshma