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