From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 03 Sep 2014 21:46:25 GMT
Subject: Re: [PATCH] parse-options: detect attempt to add a duplicate short option name
Message-ID: <20140903214624.GY18279@google.com>
In-Reply-To: <xmqqoauwwh2c.fsf@gitster.dls.corp.google.com>

Junio C Hamano wrote:

> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -345,12 +345,27 @@ static void check_typos(const char *arg, const struct option *options)
>  static void parse_options_check(const struct option *opts)
>  {
>  	int err = 0;
> +	char short_opts[128];
> +
> +	memset(short_opts, '\0', sizeof(short_opts));
>  
>  	for (; opts->type != OPTION_END; opts++) {
>  		if ((opts->flags & PARSE_OPT_LASTARG_DEFAULT) &&
>  		    (opts->flags & PARSE_OPT_OPTARG))
>  			err |= optbug(opts, "uses incompatible flags "
>  					"LASTARG_DEFAULT and OPTARG");
> +		if (opts->short_name) {
> +			struct strbuf errmsg = STRBUF_INIT;
> +			if (opts->short_name < ' ' || 0x7F <= opts->short_name)
> +				strbuf_addf(&errmsg, "invalid short name (0x%02x)",
> +					    opts->short_name);
> +			else if (short_opts[opts->short_name]++)

What happens on platforms with a signed char?

With the following squashed in,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

diff --git i/parse-options.c w/parse-options.c
index f7f153a..4cc3f3e 100644
--- i/parse-options.c
+++ w/parse-options.c
@@ -361,7 +361,7 @@ static void parse_options_check(const struct option *opts)
 			if (opts->short_name < ' ' || 0x7F <= opts->short_name)
 				strbuf_addf(&errmsg, "invalid short name (0x%02x)",
 					    opts->short_name);
-			else if (short_opts[opts->short_name]++)
+			else if (short_opts[(unsigned char) opts->short_name]++)
 				strbuf_addf(&errmsg, "short name %c already used",
 					    opts->short_name);
 			if (errmsg.len)