> -----Original Message-----
> From: Pattan, Reshma
> Sent: Tuesday, May 31, 2016 3:50 PM
> To: Ananyev, Konstantin; dev at 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 <reshma.pattan at intel.com>; dev at dpdk.org
> > Cc: Pattan, Reshma <reshma.pattan at intel.com>
> > 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 = extra_args;
> > > +
> > > + n = atoi(value);
> > > + if (n > 1024)
> > > +         pt->total_num_mbufs = (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 >= 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 later 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.

What for?

> 
> Thanks,
> Reshma

Reply via email to