2014-03-01 3:42 GMT+08:00 Junio C Hamano <gitster@xxxxxxxxx>: > 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. > OK, got it. Thanks. >> 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? The comments in parse-options.h (Line 10-16) says that CMDMODE is not marked to allow arguments. And I am not sure if removing OPTION_NUMBER is a proper way. -- 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