patchparse-options.c:parse_options_check() change OPTION_NUMBER to OPTION_CMDMODE

> Signed-off-by: Sun He <sunheehnus@gmail.com>
> ---
>  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.
> 
> According to the information the program says (Should not accept an argument), I think you should take this patch into consideration.
> Thanks.
> 
> He Sun

>
> On 02/28/2014 10:07 AM, Sun He wrote:
> > Signed-off-by: Sun He <sunheehnus@gmail.com>
> > ---
> >  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.
> >
> > According to the information the program says (Should not accept an argument), I think you should take this patch into consideration.
> > Thanks.
> >
> > He Sun
>
> Please resubmit this change in the proper format (as described by Eric
> Sunshine WRT another one of your patches).  Also please remember to
> distinguish between information that should go in the commit log
> message, which will be stored permanently to the repository and help
> future developers understand your change, vs. side notes meant only for
> the mailing list.  For example, for the log message, stuff like:
>
>     OPTION_CMDMODE should be used when ... So change the mode to
>     OPTION_CMDMODE so that ...
>
> vs. for the mailing list discussion:
>
>     When I was reading code yesterday ...
>
> The former goes above the "---" line and the latter goes directly below it.
>
> BTW, none of my comments should be taken to indicate whether the commit
> itself makes sense or not.  I haven't checked that far.
>
> Michael
>
> --
> Michael Haggerty
> mhagger@alum.mit.edu
> http://softwareswirl.blogspot.com/
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

> On 02/28/2014 10:07 AM, Sun He wrote:
>> Signed-off-by: Sun He <sunheehnus@gmail.com>
>> ---
>>  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.

> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> On 02/28/2014 10:07 AM, Sun He wrote:
>>> Signed-off-by: Sun He <sunheehnus@gmail.com>
>>> ---
>>>  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?