[PATCH v2 05/11] parse-options.c: use exhaustive "case" arms for "enum parse_opt_type"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Change code in get_value(), parse_options_check() etc. to do away with
the "default" case in favor of exhaustively checking the relevant
fields.

The added "return -1" is needed for the GCC version commented on
inline, my local clang 11.0.1-2 does not require it. Let's add it for
now to appease GCC.

The added "special types" etc. comments correspond to the relevant
comments and ordering on the "enum parse_opt_type". Let's try to keep
the same order and commentary as there where possible for
clarity. This doesn't reach that end-state, and due to the different
handling of options it's probably not worth it to get there, but let's
match its ordering where it's easy to do so.

There was a discussion about whether this was worth the added
verbosity, as argued in[1] I think it's worth it for getting
compile-time checking when adding new option types. We *should* have
tests for some of these, but e.g. in the show_gitcomp() case one might
run through the whole test suite and only hit a missing case at the
end on the completion tests.

This technically changes the handling of OPTION_END, but it's
obviously the right thing to do. We're calling this code from within a
loop that uses OPTION_END as a break condition, so it was never caught
by the "default" case.

So let's make encountering OPTION_END a BUG(), just like it already is
in the get_value() handling added in 4a59fd13122 (Add a simple option
parser., 2007-10-15).

1. https://lore.kernel.org/git/87tui3vk8y.fsf@xxxxxxxxxxxxxxxxxxx/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
 parse-options.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
 parse-options.h |  2 +-
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index e33700d6e71..dedd40efec5 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -219,9 +219,14 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
 				     optname(opt, flags));
 		return 0;
 
-	default:
+	/* special types */
+	case OPTION_END:
+	case OPTION_GROUP:
+	case OPTION_NUMBER:
+	case OPTION_ALIAS:
 		BUG("opt->type %d should not happen", opt->type);
 	}
+	return -1; /* gcc 10.2.1-6's -Werror=return-type */
 }
 
 static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
@@ -443,6 +448,9 @@ static void parse_options_check(const struct option *opts)
 			err |= optbug(opts, "uses feature "
 					"not supported for dashless options");
 		switch (opts->type) {
+		case OPTION_END:
+			BUG("unreachable");
+
 		case OPTION_COUNTUP:
 		case OPTION_BIT:
 		case OPTION_NEGBIT:
@@ -468,8 +476,14 @@ static void parse_options_check(const struct option *opts)
 			BUG("OPT_ALIAS() should not remain at this point. "
 			    "Are you using parse_options_step() directly?\n"
 			    "That case is not supported yet.");
-		default:
-			; /* ok. (usually accepts an argument) */
+
+		case OPTION_BITOP:
+		case OPTION_FILENAME:
+		case OPTION_GROUP:
+		case OPTION_INTEGER:
+		case OPTION_MAGNITUDE:
+		case OPTION_STRING:
+			break;
 		}
 		if (opts->argh &&
 		    strcspn(opts->argh, " _") != strlen(opts->argh))
@@ -532,6 +546,9 @@ static void show_negated_gitcomp(const struct option *opts, int show_all,
 			continue;
 
 		switch (opts->type) {
+		case OPTION_END:
+			BUG("unreachable");
+
 		case OPTION_STRING:
 		case OPTION_FILENAME:
 		case OPTION_INTEGER:
@@ -543,7 +560,14 @@ static void show_negated_gitcomp(const struct option *opts, int show_all,
 		case OPTION_SET_INT:
 			has_unset_form = 1;
 			break;
-		default:
+		/* special types */
+		case OPTION_GROUP:
+		case OPTION_NUMBER:
+		case OPTION_ALIAS:
+		/* options with no arguments */
+		case OPTION_BITOP:
+		/* options with arguments (usually) */
+		case OPTION_LOWLEVEL_CALLBACK:
 			break;
 		}
 		if (!has_unset_form)
@@ -578,6 +602,8 @@ static int show_gitcomp(const struct option *opts, int show_all)
 			continue;
 
 		switch (opts->type) {
+		case OPTION_END:
+			BUG("unreachable");
 		case OPTION_GROUP:
 			continue;
 		case OPTION_STRING:
@@ -593,7 +619,19 @@ static int show_gitcomp(const struct option *opts, int show_all)
 				break;
 			suffix = "=";
 			break;
-		default:
+		/* special types */
+		case OPTION_NUMBER:
+		case OPTION_ALIAS:
+
+		/* options with no arguments */
+		case OPTION_BIT:
+		case OPTION_NEGBIT:
+		case OPTION_BITOP:
+		case OPTION_COUNTUP:
+		case OPTION_SET_INT:
+
+		/* options with arguments (usually) */
+		case OPTION_LOWLEVEL_CALLBACK:
 			break;
 		}
 		if (opts->flags & PARSE_OPT_COMP_ARG)
diff --git a/parse-options.h b/parse-options.h
index d931300f4d6..a1c7c86ad30 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -264,7 +264,7 @@ struct parse_opt_ctx_t {
 	const char **out;
 	int argc, cpidx, total;
 	const char *opt;
-	int flags;
+	enum parse_opt_flags flags;
 	const char *prefix;
 	const char **alias_groups; /* must be in groups of 3 elements! */
 	struct option *updated_options;
-- 
2.33.0.1374.gc8f4fa74caf




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux