Michael Haggerty <mhagger@xxxxxxxxxxxx> writes: > On 02/28/2014 10:07 AM, Sun He wrote: >> Signed-off-by: Sun He <sunheehnus@xxxxxxxxx> >> --- >> parse-options.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/parse-options.c b/parse-options.c >> index 7b8d3fa..59a52b0 100644 >> --- a/parse-options.c >> +++ b/parse-options.c >> @@ -371,7 +371,7 @@ static void parse_options_check(const struct option *opts) >> case OPTION_NEGBIT: >> case OPTION_SET_INT: >> case OPTION_SET_PTR: >> - case OPTION_NUMBER: >> + case OPTION_CMDMODE: >> if ((opts->flags & PARSE_OPT_OPTARG) || >> !(opts->flags & PARSE_OPT_NOARG)) >> err |= optbug(opts, "should not accept an argument"); >> >> -- >> 1.9.0.138.g2de3478.dirty >> >> Hi, >> When I was reading code yesterday, I came across this protential bug. >> parse-options.h says that OPTION_CMDMODE is an option with no arguments and OPTION_NUMBER is special type option. Please, no overlong line in the text part that makes things unnecessarily hard to read. I agree with all the comments in the message I am responding to. > BTW, none of my comments should be taken to indicate whether the commit > itself makes sense or not. I haven't checked that far. While I think it is sensible to make sure CMDMODE is not marked to allow arguments, I do not understand why "special type" would imply "it is allowed to be marked to take an argument". Why is it a good change to remove "case OPTION_NUMBER:" here? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html