Re: [PATCH] parse-options.c:parse_options_check() change OPTION_NUMBER to OPTION_CMDMODE

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Feb 28, 2014, 10:15 UTC
Message-ID: <531061D1.2060206@alum.mit.edu>
In-Reply-To: <1393578442-2822-1-git-send-email-sunheehnus@gmail.com>

On 02/28/2014 10:07 AM, Sun He wrote:

Show 30 quoted lines

> 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/