Re: [PATCH] parse-options: detect attempt to add a duplicate short option name

[Jonathan Nieder]

On Wed, Sep 03, 2014 at 02:46:25PM -0700, Jonathan Nieder wrote:
> 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?

Ah, I see now that the "< ' '" check would catch that.

With René's suggestions squashed in, that becomes

 parse-options.c               | 9 +++++++++
 t/t1502-rev-parse-parseopt.sh | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index e7dafa8..6ad7d90 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -347,12 +347,21 @@ 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) {
+                       if (opts->short_name <= ' ' || 0x7F <= opts->short_name)
+                               err |= optbug(opts, "invalid short name");
+                       else if (short_opts[opts->short_name]++)
+                               err |= optbug(opts, "short name already used");
+               }
                if (opts->flags & PARSE_OPT_NODASH &&
                    ((opts->flags & PARSE_OPT_OPTARG) ||
                     !(opts->flags & PARSE_OPT_NOARG) ||
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 922423e..ebe7c3b 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -19,7 +19,7 @@ sed -e 's/^|//' >expect <<\END_EXPECT
 |    -d, --data[=...]      short and long option with an optional argument
 |
 |Argument hints
-|    -b <arg>              short option required argument
+|    -B <arg>              short option required argument
 |    --bar2 <arg>          long option required argument
 |    -e, --fuz <with-space>
 |                          short and long option required argument
@@ -51,7 +51,7 @@ sed -e 's/^|//' >optionspec <<\EOF
 |d,data?   short and long option with an optional argument
 |
 | Argument hints
-|b=arg     short option required argument
+|B=arg     short option required argument
 |bar2=arg  long option required argument
 |e,fuz=with-space  short and long option required argument
 |s?some    short option optional argument
-- 
2.1.0.rc2.206.gedb03e5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html