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

[David Marchand <david.marchand@6wind.com>]

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.


[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 ?


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

-- 
David Marchand