git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC PATCH 09/21] parse-options.[ch] API: use bug() to improve error output

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 15, 2021, 22:18 UTC
Message-ID
<RFC-patch-09.21-9c6af87c6c9-20211115T220831Z-avarab@gmail.com>
In-Reply-To
<RFC-cover-00.21-00000000000-20211115T220831Z-avarab@gmail.com>

When we run into bugs in parse-options.c usage it's good to be able to note all the issues we ran into before dying, which is why we have the optbug() function.

Let's instead use the bug() helper function that's newly added to usage.c to do the same thing, which cuts down on the verbosity of parse_options_check().

In addition change the use of BUG() in that function to bug(), we'll be dying soon enough, but always want exhaustive error reporting from the function.

Let's also use bug() instead of BUG() in preprocess_options() and parse_options_start_1() (which is called shortly after preprocess_options()). Since the BUG_if_bug() is called at the end of parse_options_start_1() we won't miss it, and even if we did the invocation in common-main.c would trigger.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 parse-options.c | 54 ++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 8bc0a21f1d7..54cbd382cb5 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -14,15 +14,15 @@ enum opt_parsed {
 	OPT_UNSET = 1<<1,
 };
 
-static int optbug(const struct option *opt, const char *reason)
+static void optbug(const struct option *opt, const char *reason)
 {
-	if (opt->long_name) {
-		if (opt->short_name)
-			return error("BUG: switch '%c' (--%s) %s",
-				     opt->short_name, opt->long_name, reason);
-		return error("BUG: option '%s' %s", opt->long_name, reason);
-	}
-	return error("BUG: switch '%c' %s", opt->short_name, reason);
+	if (opt->long_name && opt->short_name)
+		bug("switch '%c' (--%s) %s", opt->short_name,
+		    opt->long_name, reason);
+	else if (opt->long_name)
+		bug("option '%s' %s", opt->long_name, reason);
+	else
+		bug("switch '%c' %s", opt->short_name, reason);
 }
 
 static const char *optname(const struct option *opt, enum opt_parsed flags)
@@ -440,28 +440,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");
+			optbug(opts, "uses incompatible flags "
+			       "LASTARG_DEFAULT and OPTARG");
 		if (opts->short_name) {
 			if (0x7F <= opts->short_name)
-				err |= optbug(opts, "invalid short name");
+				optbug(opts, "invalid short name");
 			else if (short_opts[opts->short_name]++)
-				err |= optbug(opts, "short name already used");
+				optbug(opts, "short name already used");
 		}
 		if (opts->flags & PARSE_OPT_NODASH &&
 		    ((opts->flags & PARSE_OPT_OPTARG) ||
 		     !(opts->flags & PARSE_OPT_NOARG) ||
 		     !(opts->flags & PARSE_OPT_NONEG) ||
 		     opts->long_name))
-			err |= optbug(opts, "uses feature "
-					"not supported for dashless options");
+			optbug(opts, "uses feature "
+			       "not supported for dashless options");
 		switch (opts->type) {
 		case OPTION_COUNTUP:
 		case OPTION_BIT:
@@ -470,22 +469,22 @@ static void parse_options_check(const struct option *opts)
 		case OPTION_NUMBER:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
 			    !(opts->flags & PARSE_OPT_NOARG))
-				err |= optbug(opts, "should not accept an argument");
+				optbug(opts, "should not accept an argument");
 			break;
 		case OPTION_CALLBACK:
 			if (!opts->callback && !opts->ll_callback)
-				BUG("OPTION_CALLBACK needs one callback");
+				bug("OPTION_CALLBACK needs one callback");
 			if (opts->callback && opts->ll_callback)
-				BUG("OPTION_CALLBACK can't have two callbacks");
+				bug("OPTION_CALLBACK can't have two callbacks");
 			break;
 		case OPTION_LOWLEVEL_CALLBACK:
 			if (!opts->ll_callback)
-				BUG("OPTION_LOWLEVEL_CALLBACK needs a callback");
+				bug("OPTION_LOWLEVEL_CALLBACK needs a callback");
 			if (opts->callback)
-				BUG("OPTION_LOWLEVEL_CALLBACK needs no high level callback");
+				bug("OPTION_LOWLEVEL_CALLBACK needs no high level callback");
 			break;
 		case OPTION_ALIAS:
-			BUG("OPT_ALIAS() should not remain at this point. "
+			bug("OPT_ALIAS() should not remain at this point. "
 			    "Are you using parse_options_step() directly?\n"
 			    "That case is not supported yet.");
 		default:
@@ -493,10 +492,8 @@ static void parse_options_check(const struct option *opts)
 		}
 		if (opts->argh &&
 		    strcspn(opts->argh, " _") != strlen(opts->argh))
-			err |= optbug(opts, "multi-word argh should use dash to separate words");
+			optbug(opts, "multi-word argh should use dash to separate words");
 	}
-	if (err)
-		exit(128);
 }
 
 static void parse_options_start_1(struct parse_opt_ctx_t *ctx,
@@ -518,11 +515,12 @@ static void parse_options_start_1(struct parse_opt_ctx_t *ctx,
 	if ((flags & PARSE_OPT_KEEP_UNKNOWN) &&
 	    (flags & PARSE_OPT_STOP_AT_NON_OPTION) &&
 	    !(flags & PARSE_OPT_ONE_SHOT))
-		BUG("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");
+		bug("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");
 	if ((flags & PARSE_OPT_ONE_SHOT) &&
 	    (flags & PARSE_OPT_KEEP_ARGV0))
-		BUG("Can't keep argv0 if you don't have it");
+		bug("Can't keep argv0 if you don't have it");
 	parse_options_check(options);
+	BUG_if_bug();
 }
 
 void parse_options_start(struct parse_opt_ctx_t *ctx,
@@ -673,7 +671,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
 		source = newopt[i].value;
 
 		if (!long_name)
-			BUG("An alias must have long option name");
+			bug("An alias must have long option name");
 		strbuf_addf(&help, _("alias of --%s"), source);
 
 		for (j = 0; j < nr; j++) {
@@ -694,7 +692,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
 		}
 
 		if (j == nr)
-			BUG("could not find source option '%s' of alias '%s'",
+			bug("could not find source option '%s' of alias '%s'",
 			    source, newopt[i].long_name);
 		ctx->alias_groups[alias * 3 + 0] = newopt[i].long_name;
 		ctx->alias_groups[alias * 3 + 1] = options[j].long_name;
-- 
2.34.0.rc2.809.g11e21d44b24
Previous: Ævar Arnfjörð BjarmasonNext: Taylor Blau
Message 30 of 34 in “C99: show meaningful <file>:<line> in trace2 via macros”
  1. 00/21 C99: show meaningful <file>:<line> in trace2 via macrosÆvar Arnfjörð Bjarmason, Nov 15, 2021
  2. 01/21 git-compat-util.h: clarify GCC v.s. C99-specific in commentÆvar Arnfjörð Bjarmason, Nov 15, 2021
  3. 03/21 usage.c: add a die_message() routineÆvar Arnfjörð Bjarmason, Nov 15, 2021
  4. 02/21 C99 support: hard-depend on C99 variadic macrosÆvar Arnfjörð Bjarmason, Nov 15, 2021
  5. 04/21 usage.c API users: use die_message() where appropriateÆvar Arnfjörð Bjarmason, Nov 15, 2021
  6. 05/21 usage.c + gc: add and use a die_message_errno()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  7. 07/21 common-main.c: call exit(), don't returnÆvar Arnfjörð Bjarmason, Nov 15, 2021
  8. 06/21 config API: don't use vreportf(), make it static in usage.cÆvar Arnfjörð Bjarmason, Nov 15, 2021
  9. 08/21 usage.c: add a non-fatal bug() function to go with BUG()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  10. 10/21 receive-pack: use bug() and BUG_if_bug()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  11. 11/21 cache-tree.c: use bug() and BUG_if_bug()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  12. 12/21 pack-objects: use BUG(...) not die("BUG: ...")Ævar Arnfjörð Bjarmason, Nov 15, 2021
  13. 17/21 usage.c: move usage routines aroundÆvar Arnfjörð Bjarmason, Nov 15, 2021
  14. 21/21 usage API: add "core.usageAddSource" config to add <file>:<line>Ævar Arnfjörð Bjarmason, Nov 15, 2021
  15. 20/21 usage API: make the "{usage,fatal,error,warning,BUG}: " translatableÆvar Arnfjörð Bjarmason, Nov 15, 2021
  16. 19/21 usage API: use C99 macros for {usage,usagef,die,error,warning,die}*()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  17. Jeff HostetlerDec 27, 2021
  18. Ævar Arnfjörð BjarmasonDec 27, 2021
  19. Jeff HostetlerDec 28, 2021
  20. Elijah NewrenDec 28, 2021
  21. Ævar Arnfjörð BjarmasonDec 28, 2021
  22. Elijah NewrenDec 29, 2021
  23. Ævar Arnfjörð BjarmasonDec 28, 2021
  24. Jeff HostetlerDec 29, 2021
  25. 18/21 usage.c: move rename variables in usage routines aroundÆvar Arnfjörð Bjarmason, Nov 15, 2021
  26. 16/21 usage.[ch] API: rename "warn" vars functions to "warning"Ævar Arnfjörð Bjarmason, Nov 15, 2021
  27. 14/21 usage API: create a new usage.h, move API docs thereÆvar Arnfjörð Bjarmason, Nov 15, 2021
  28. 15/21 usage.[ch] API users: use report_fn, not hardcoded prototypeÆvar Arnfjörð Bjarmason, Nov 15, 2021
  29. 13/21 strbuf.h: use BUG(...) not die("BUG: ...")Ævar Arnfjörð Bjarmason, Nov 15, 2021
  30. 09/21 parse-options.[ch] API: use bug() to improve error outputÆvar Arnfjörð Bjarmason, Nov 15, 2021
  31. Taylor BlauNov 16, 2021
  32. Ævar Arnfjörð BjarmasonNov 16, 2021
  33. Taylor BlauNov 16, 2021
  34. Ævar Arnfjörð BjarmasonNov 16, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.