Re: [dpdk-dev] [PATCH v2 1/2] eal: sort and align options lists

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2015-02-03 07:26, David Marchand:
> Two little comments.
> 
> On Mon, Feb 2, 2015 at 6:44 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
> wrote:
> > @@ -578,37 +579,36 @@ eal_check_common_options(struct internal_config
> > *internal_cfg)
> >  void
> >  eal_common_usage(void)
> >  {
> > -       printf("-c COREMASK -n NUM [-m NB] [-r NUM] [-b <domain:bus:devid.func>]"
> > -              "[--proc-type primary|secondary|auto]\n\n"
> > +       printf("-c COREMASK|-l CORELIST -n CHANNELS [options]\n\n"
> >                "EAL common options:\n"
> > -              "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
> > -              "  -l CORELIST  : List of cores to run on\n"
> > -              "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
> [snip]
> > +              "  -n NUM              Number of memory channels\n"
> 
> Not really a problem, but for consistency : here, you are talking about
> NUM, while at first, you wrote -n CHANNELS.

Yes you're right. I changed headline but not the description of this option.
Will do.

> [snip]
> >         /* first long only option value must be >= 256, so that we won't
> >          * conflict with short options */
> >         OPT_LONG_MIN_NUM = 256,
> > -#define OPT_HUGE_DIR    "huge-dir"
> > -       OPT_HUGE_DIR_NUM = OPT_LONG_MIN_NUM,
> > -#define OPT_MASTER_LCORE "master-lcore"
> > +#define OPT_BASE_VIRTADDR     "base-virtaddr"
> > +       OPT_BASE_VIRTADDR_NUM,
> 
> Why skip the first entry ?
> Afaik, OPT_BASE_VIRTADDR_NUM will be set to 257, is it to avoid having this
> = OPT_LONG_MIN_NUM moved anytime we add a new long option at the top of the
> enum ?

Exactly, yes. I think we don't care what is the first number.
It doesn't deserve a painful assignment.

> The rest looks good to me.
> Acked-by: David Marchand <david.marchand@6wind.com>

Thanks
-- 
Thomas