@matheusbsbs / matheus-bagliano / commits / af465af8de

parse-options: detect attempt to add a duplicate short option name

It is easy to overlook an already assigned single-letter option name and try to use it for a new one. Help the developer to catch it before such a mistake escapes the lab. This retroactively forbids any short option name (which is defined to be of type "int") outside the ASCII printable range. We might want to do one of two things: - tighten the type of short_name member to 'char', and further update optbug() to protect it against doing "'%c'" on a funny value, e.g. negative or above 127. - drop the check (even the "duplicate" check) for an option whose short_name is either negative or above 255, to allow clever folks to take advantage of the fact that such a short_name cannot be parsed from the command line and the member can be used to store some extra information. Helped-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Sep 3, 2014 at 12:42 UTC af465af8deda8ad685f7704a2dc787845eb317ed

2 files changed +15 -3

parse-options.c

+13 -1

@@ -14,8 +14,12 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,

15 int optbug(const struct option *opt, const char *reason)

16 {

17 - if (opt->long_name)

17 + if (opt->long_name) {

18 + if (opt->short_name)

19 + return error("BUG: switch '%c' (--%s) %s",

20 + opt->short_name, opt->long_name, reason);

21 return error("BUG: option '%s' %s", opt->long_name, reason);

22 + }

23 return error("BUG: switch '%c' %s", opt->short_name, reason);

24 }

@@ -345,12 +349,20 @@ static void check_typos(const char *arg, const struct option *options)

349 static void parse_options_check(const struct option *opts)

350 {

351 int err = 0;

352 + char short_opts[128];

353

354 + memset(short_opts, '\0', sizeof(short_opts));

355 for (; opts->type != OPTION_END; opts++) {

356 if ((opts->flags & PARSE_OPT_LASTARG_DEFAULT) &&

357 (opts->flags & PARSE_OPT_OPTARG))

358 err |= optbug(opts, "uses incompatible flags "

359 "LASTARG_DEFAULT and OPTARG");

360 + if (opts->short_name) {

361 + if (0x7F <= opts->short_name)

362 + err |= optbug(opts, "invalid short name");

363 + else if (short_opts[opts->short_name]++)

364 + err |= optbug(opts, "short name already used");

365 + }

366 if (opts->flags & PARSE_OPT_NODASH &&

367 ((opts->flags & PARSE_OPT_OPTARG) ||

368 !(opts->flags & PARSE_OPT_NOARG) ||

t/t1502-rev-parse-parseopt.sh

+2 -2

@@ -19,7 +19,7 @@ sed -e 's/^|//' >expect <<\END_EXPECT

19 | -d, --data[=...] short and long option with an optional argument

20 |

21 |Argument hints

22 -| -b <arg> short option required argument

22 +| -B <arg> short option required argument

23 | --bar2 <arg> long option required argument

24 | -e, --fuz <with-space>

25 | short and long option required argument

@@ -51,7 +51,7 @@ sed -e 's/^|//' >optionspec <<\EOF

51 |d,data? short and long option with an optional argument

52 |

53 | Argument hints

54 -|b=arg short option required argument

54 +|B=arg short option required argument

55 |bar2=arg long option required argument

56 |e,fuz=with-space short and long option required argument

57 |s?some short option optional argument