[10/10] parse-options: change OPT_{SHORT,UNSET} to an enum

Commit Message

Ævar Arnfjörð Bjarmason Sept. 28, 2021, 1:14 p.m. UTC

Change the comparisons against OPT_SHORT and OPT_UNSET to an enum
which keeps track of how a given option got parsed. The case of "0"
was an implicit OPT_LONG, so let's add an explicit label for it.

Due to the xor in 0f1930c5875 (parse-options: allow positivation of
options starting, with no-, 2012-02-25) the code already relied on
this being set back to 0. To avoid refactoring the logic involved in
that let's just start the enum at "0" instead of the usual "1<<0" (1),
but BUG() out if we don't have one of our expected flags.

Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
---
 parse-options.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

Comments

Junio C Hamano Sept. 29, 2021, 12:50 a.m. UTC | #1

Ævar Arnfjörð Bjarmason  <[e-mail]> writes:

> Change the comparisons against OPT_SHORT and OPT_UNSET to an enum
> which keeps track of how a given option got parsed. The case of "0"
> was an implicit OPT_LONG, so let's add an explicit label for it.

I think this is going backwards.  If we want to change anything in
this area, you may

 (1) spell the values not as 1 and 2 but as bit-shifts (1U<<0),
     (1U<<1), and

 (2) turn "flags" into "unsigned".

but I do not think of a reason why it would be a good move to use
enum for "these are bits", for the same reason as my comments on
[2/10].

Thanks.

Patch

diff --git a/parse-options.c b/parse-options.c
index 57e95846e83..1eb3b51f753 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -8,8 +8,11 @@ 
 
 static int disallow_abbreviated_options;
 
-#define OPT_SHORT 1
-#define OPT_UNSET 2
+enum opt_parsed {
+	OPT_LONG  = 0,
+	OPT_SHORT = 1<<0,
+	OPT_UNSET = 1<<1,
+};
 
 static int optbug(const struct option *opt, const char *reason)
 {
@@ -22,7 +25,7 @@  static int optbug(const struct option *opt, const char *reason)
 	return error("BUG: switch '%c' %s", opt->short_name, reason);
 }
 
-static const char *optname(const struct option *opt, int flags)
+static const char *optname(const struct option *opt, enum opt_parsed flags)
 {
 	static struct strbuf sb = STRBUF_INIT;
 
@@ -31,15 +34,17 @@  static const char *optname(const struct option *opt, int flags)
 		strbuf_addf(&sb, "switch `%c'", opt->short_name);
 	else if (flags & OPT_UNSET)
 		strbuf_addf(&sb, "option `no-%s'", opt->long_name);
-	else
+	else if (flags == OPT_LONG)
 		strbuf_addf(&sb, "option `%s'", opt->long_name);
+	else
+		BUG("optname() got unknown flags %d", flags);
 
 	return sb.buf;
 }
 
 static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
 				     const struct option *opt,
-				     int flags, const char **arg)
+				     enum opt_parsed flags, const char **arg)
 {
 	if (p->opt) {
 		*arg = p->opt;
@@ -65,7 +70,7 @@  static void fix_filename(const char *prefix, const char **file)
 static enum parse_opt_result opt_command_mode_error(
 	const struct option *opt,
 	const struct option *all_opts,
-	int flags)
+	enum opt_parsed flags)
 {
 	const struct option *that;
 	struct strbuf that_name = STRBUF_INIT;
@@ -97,7 +102,7 @@  static enum parse_opt_result opt_command_mode_error(
 static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
 				       const struct option *opt,
 				       const struct option *all_opts,
-				       int flags)
+				       enum opt_parsed flags)
 {
 	const char *s, *arg;
 	const int unset = flags & OPT_UNSET;
@@ -318,11 +323,11 @@  static enum parse_opt_result parse_long_opt(
 	const struct option *all_opts = options;
 	const char *arg_end = strchrnul(arg, '=');
 	const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
-	int abbrev_flags = 0, ambiguous_flags = 0;
+	enum opt_parsed abbrev_flags = OPT_LONG, ambiguous_flags = OPT_LONG;
 
 	for (; options->type != OPTION_END; options++) {
 		const char *rest, *long_name = options->long_name;
-		int flags = 0, opt_flags = 0;
+		enum opt_parsed flags = OPT_LONG, opt_flags = OPT_LONG;
 
 		if (!long_name)
 			continue;