2014-11-05 11:54, Ananyev, Konstantin:
> From: Thomas Monjalon
> > + long master_lcore;
> > + char *parsing_end;
> > + struct rte_config *cfg = rte_eal_get_configuration();
> > +
> > + errno = 0;
> > + master_lcore = strtol(arg, &parsing_end, 0);
> > + if (errno || parsing_end == arg)
> > + return -1;
>
> Why not: "errno || parsing_end[0] != 0"
> ?
> Otherwise something like "1blah" would be considered as valid input.
Good point.
> > + if (!(master_lcore >= 0 && master_lcore < RTE_MAX_LCORE))
> > + return -1;
>
> If negative values are not allowed, then why not:
>
> unsigned long master_lcore;
> ...
> master_lcore = strtoul(...)
> ...
> if(master_clore > RTE_MAX_LCORE)
> return -1;
Matter of taste. Your code is less explicit.
But it should be
if(master_clore >= RTE_MAX_LCORE)
Anyone else to vote for 1 solution or the other?
> > + if (opt == OPT_MASTER_LCORE_NUM && !coremask_ok) {
> > + RTE_LOG(ERR, EAL, "please specify the master lcore id"
> > + "after specifying the coremask\n");
> > + eal_usage(prgname);
> > + return -1;
> > + }
> > +
>
> I don't really like an idea of introducing strict order between -c and
> "--master-lcore..
Me too. And Aaron too :)
> Can we move check for coremask_ok/ and assignment of cfg->master_lcore out of
>
> while (getopt_long(...)) loop?
>
> > ret = eal_parse_common_option(opt, optarg, &internal_config);
> > /* common parser is not happy */
> > if (ret < 0) {
Yes we should move the check outside of the loop.
First we should migrate all flags check in a common function for BSD and Linux.
Simon made the v1. I made the v2. Any volunteer for the v3?
--
Thomas
