Re: [PATCH 04/12] mkfs: merge tables for opts parsing into one table

Eric Sandeen
2017-04-25T21:45:22Z

On 4/23/17 1:54 PM, Jan Tulak wrote:
> Merge separate instances of opt_params into one indexable table. Git
> makes this patch looks a bit more complicated, but it does not change
> values or structure of anything else. It only moves all the "struct
> opt_params dopts = {...}", changes indentation for these substructures
> and replaces their usage (dopts -> opts[OPT_D]).

This looks correct. Can you remind me why it helps? :)
(the commit log says what you did but not why you did it)

One thing I wondered about to idiot-proof the code a bit (in some
other patch, not this one, but while I'm looking at it):

What about instead of:

switch (getsubopt(&p, subopts, &value)) {
case OPT1:
opt1 = getnum(value, &opts, OPT1);
case OPT2:
opt2 = getnum(value, &opts, OPT2);
...

and making sure there's never a mismatch between the case value and the
value passed to getnum, you could do something like:

int so = getsubopt(&p, subopts, &value);

switch (so) {
case OPT1:
opt1 = getnum(value, &opts, so);
case OPT2:
opt2 = getnum(value, &opts, so);
...

so that you can never mismatch & pass in the wrong subopt for option
checking; it will always be the subopt matched by the case statement.

I just considered that as I reviewed it and found myself mentally
checking that each case matched.

Thanks,
-Eric