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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux