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

[PATCH 07/76] parse-options: avoid magic return codes

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Jan 17, 2019, 13:05 UTC
Message-ID
<20190117130615.18732-8-pclouds@gmail.com>
In-Reply-To
<20190117130615.18732-1-pclouds@gmail.com>

Give names to these magic negative numbers. Make parse_opt_ll_cb return an enum to make clear it can actually control parse_options() with different return values (parse_opt_cb can too, but nobody needs it).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/merge.c        |  5 +--
 builtin/update-index.c | 20 ++++++------
 parse-options-cb.c     |  6 ++--
 parse-options.c        | 69 +++++++++++++++++++++++++++---------------
 parse-options.h        | 14 ++++-----
 5 files changed, 68 insertions(+), 46 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 07839b0bb8..de64d7850e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -112,8 +112,9 @@ static int option_parse_message(const struct option *opt,
 	return 0;
 }
 
-static int option_read_message(struct parse_opt_ctx_t *ctx,
-			       const struct option *opt, int unset)
+static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
+						 const struct option *opt,
+						 int unset)
 {
 	struct strbuf *buf = opt->value;
 	const char *arg;
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 727a8118b8..21c84e5590 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -847,8 +847,8 @@ static int parse_new_style_cacheinfo(const char *arg,
 	return 0;
 }
 
-static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
-				const struct option *opt, int unset)
+static enum parse_opt_result cacheinfo_callback(
+	struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
 {
 	struct object_id oid;
 	unsigned int mode;
@@ -873,8 +873,8 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
 	return 0;
 }
 
-static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
-			      const struct option *opt, int unset)
+static enum parse_opt_result stdin_cacheinfo_callback(
+	struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
 {
 	int *nul_term_line = opt->value;
 
@@ -887,8 +887,8 @@ static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
 	return 0;
 }
 
-static int stdin_callback(struct parse_opt_ctx_t *ctx,
-				const struct option *opt, int unset)
+static enum parse_opt_result stdin_callback(
+	struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
 {
 	int *read_from_stdin = opt->value;
 
@@ -900,8 +900,8 @@ static int stdin_callback(struct parse_opt_ctx_t *ctx,
 	return 0;
 }
 
-static int unresolve_callback(struct parse_opt_ctx_t *ctx,
-				const struct option *opt, int unset)
+static enum parse_opt_result unresolve_callback(
+	struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
 {
 	int *has_errors = opt->value;
 	const char *prefix = startup_info->prefix;
@@ -919,8 +919,8 @@ static int unresolve_callback(struct parse_opt_ctx_t *ctx,
 	return 0;
 }
 
-static int reupdate_callback(struct parse_opt_ctx_t *ctx,
-				const struct option *opt, int unset)
+static enum parse_opt_result reupdate_callback(
+	struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
 {
 	int *has_errors = opt->value;
 	const char *prefix = startup_info->prefix;
diff --git a/parse-options-cb.c b/parse-options-cb.c
index e05bcea809..ec01ef722b 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -170,10 +170,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
  * "-h" output even if it's not being handled directly by
  * parse_options().
  */
-int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
-			 const struct option *opt, int unset)
+enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
+					   const struct option *opt, int unset)
 {
-	return -2;
+	return PARSE_OPT_UNKNOWN;
 }
 
 /**
diff --git a/parse-options.c b/parse-options.c
index ac109e11e9..372f5cede4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -20,8 +20,9 @@ int optbug(const struct option *opt, const char *reason)
 	return error("BUG: switch '%c' %s", opt->short_name, reason);
 }
 
-static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
-		   int flags, const char **arg)
+static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
+				     const struct option *opt,
+				     int flags, const char **arg)
 {
 	if (p->opt) {
 		*arg = p->opt;
@@ -44,9 +45,10 @@ static void fix_filename(const char *prefix, const char **file)
 	*file = prefix_filename(prefix, *file);
 }
 
-static int opt_command_mode_error(const struct option *opt,
-				  const struct option *all_opts,
-				  int flags)
+static enum parse_opt_result opt_command_mode_error(
+	const struct option *opt,
+	const struct option *all_opts,
+	int flags)
 {
 	const struct option *that;
 	struct strbuf that_name = STRBUF_INIT;
@@ -69,16 +71,16 @@ static int opt_command_mode_error(const struct option *opt,
 		error(_("%s is incompatible with %s"),
 		      optname(opt, flags), that_name.buf);
 		strbuf_release(&that_name);
-		return -1;
+		return PARSE_OPT_ERROR;
 	}
 	return error(_("%s : incompatible with something else"),
 		     optname(opt, flags));
 }
 
-static int get_value(struct parse_opt_ctx_t *p,
-		     const struct option *opt,
-		     const struct option *all_opts,
-		     int flags)
+static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
+				       const struct option *opt,
+				       const struct option *all_opts,
+				       int flags)
 {
 	const char *s, *arg;
 	const int unset = flags & OPT_UNSET;
@@ -208,7 +210,8 @@ static int get_value(struct parse_opt_ctx_t *p,
 	}
 }
 
-static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
+static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
+					     const struct option *options)
 {
 	const struct option *all_opts = options;
 	const struct option *numopt = NULL;
@@ -239,11 +242,12 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
 		free(arg);
 		return rc;
 	}
-	return -2;
+	return PARSE_OPT_UNKNOWN;
 }
 
-static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
-			  const struct option *options)
+static enum parse_opt_result parse_long_opt(
+	struct parse_opt_ctx_t *p, const char *arg,
+	const struct option *options)
 {
 	const struct option *all_opts = options;
 	const char *arg_end = strchrnul(arg, '=');
@@ -269,7 +273,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
 			if (*rest)
 				continue;
 			p->out[p->cpidx++] = arg - 2;
-			return 0;
+			return PARSE_OPT_DONE;
 		}
 		if (!rest) {
 			/* abbreviated? */
@@ -334,11 +338,11 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
 			ambiguous_option->long_name,
 			(abbrev_flags & OPT_UNSET) ?  "no-" : "",
 			abbrev_option->long_name);
-		return -3;
+		return PARSE_OPT_HELP;
 	}
 	if (abbrev_option)
 		return get_value(p, abbrev_option, all_opts, abbrev_flags);
-	return -2;
+	return PARSE_OPT_UNKNOWN;
 }
 
 static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
@@ -586,22 +590,28 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		if (arg[1] != '-') {
 			ctx->opt = arg + 1;
 			switch (parse_short_opt(ctx, options)) {
-			case -1:
+			case PARSE_OPT_ERROR:
 				return PARSE_OPT_ERROR;
-			case -2:
+			case PARSE_OPT_UNKNOWN:
 				if (ctx->opt)
 					check_typos(arg + 1, options);
 				if (internal_help && *ctx->opt == 'h')
 					goto show_usage;
 				goto unknown;
+			case PARSE_OPT_NON_OPTION:
+			case PARSE_OPT_HELP:
+			case PARSE_OPT_COMPLETE:
+				BUG("parse_short_opt() cannot return these");
+			case PARSE_OPT_DONE:
+				break;
 			}
 			if (ctx->opt)
 				check_typos(arg + 1, options);
 			while (ctx->opt) {
 				switch (parse_short_opt(ctx, options)) {
-				case -1:
+				case PARSE_OPT_ERROR:
 					return PARSE_OPT_ERROR;
-				case -2:
+				case PARSE_OPT_UNKNOWN:
 					if (internal_help && *ctx->opt == 'h')
 						goto show_usage;
 
@@ -613,6 +623,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 					ctx->argv[0] = xstrdup(ctx->opt - 1);
 					*(char *)ctx->argv[0] = '-';
 					goto unknown;
+				case PARSE_OPT_NON_OPTION:
+				case PARSE_OPT_COMPLETE:
+				case PARSE_OPT_HELP:
+					BUG("parse_short_opt() cannot return these");
+				case PARSE_OPT_DONE:
+					break;
 				}
 			}
 			continue;
@@ -631,12 +647,17 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		if (internal_help && !strcmp(arg + 2, "help"))
 			goto show_usage;
 		switch (parse_long_opt(ctx, arg + 2, options)) {
-		case -1:
+		case PARSE_OPT_ERROR:
 			return PARSE_OPT_ERROR;
-		case -2:
+		case PARSE_OPT_UNKNOWN:
 			goto unknown;
-		case -3:
+		case PARSE_OPT_HELP:
 			goto show_usage;
+		case PARSE_OPT_NON_OPTION:
+		case PARSE_OPT_COMPLETE:
+			BUG("parse_long_opt() cannot return these");
+		case PARSE_OPT_DONE:
+			break;
 		}
 		continue;
 unknown:
diff --git a/parse-options.h b/parse-options.h
index f1f246387c..4e49185027 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -49,8 +49,8 @@ struct option;
 typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
 
 struct parse_opt_ctx_t;
-typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
-				const struct option *opt, int unset);
+typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
+					      const struct option *opt, int unset);
 
 /*
  * `type`::
@@ -222,12 +222,12 @@ const char *optname(const struct option *opt, int flags);
 
 /*----- incremental advanced APIs -----*/
 
-enum {
-	PARSE_OPT_COMPLETE = -2,
-	PARSE_OPT_HELP = -1,
-	PARSE_OPT_DONE,
+enum parse_opt_result {
+	PARSE_OPT_COMPLETE = -3,
+	PARSE_OPT_HELP = -2,
+	PARSE_OPT_ERROR = -1,	/* must be the same as error() */
+	PARSE_OPT_DONE = 0,	/* fixed so that "return 0" works */
 	PARSE_OPT_NON_OPTION,
-	PARSE_OPT_ERROR,
 	PARSE_OPT_UNKNOWN
 };
 

-- 
2.20.0.482.g66447595a7
Previous: Nguyễn Thái Ngọc DuyNext: Nguyễn Thái Ngọc Duy
Message 9 of 88 in “Convert diff opt parser to parse_options()”
  1. 00/76 Convert diff opt parser to parse_options()Nguyễn Thái Ngọc Duy, Jan 17, 2019
  2. 01/76 parse-options.h: remove extern on function prototypesNguyễn Thái Ngọc Duy, Jan 17, 2019
  3. 02/76 parse-options: add one-shot modeNguyễn Thái Ngọc Duy, Jan 17, 2019
  4. 03/76 parse-options: allow keep-unknown + stop-at-non-opt combinationNguyễn Thái Ngọc Duy, Jan 17, 2019
  5. Stefan BellerJan 17, 2019
  6. 04/76 parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWNNguyễn Thái Ngọc Duy, Jan 17, 2019
  7. 05/76 parse-options: add OPT_BITOP()Nguyễn Thái Ngọc Duy, Jan 17, 2019
  8. 06/76 parse-options: stop abusing 'callback' for lowlevel callbacksNguyễn Thái Ngọc Duy, Jan 17, 2019
  9. 07/76 parse-options: avoid magic return codesNguyễn Thái Ngọc Duy, Jan 17, 2019
  10. 08/76 parse-options: allow ll_callback with OPTION_CALLBACKNguyễn Thái Ngọc Duy, Jan 17, 2019
  11. 09/76 diff.h: keep forward struct declarations sortedNguyễn Thái Ngọc Duy, Jan 17, 2019
  12. 10/76 diff.h: avoid bit fields in struct diff_flagsNguyễn Thái Ngọc Duy, Jan 17, 2019
  13. 11/76 diff.c: prepare to use parse_options() for parsingNguyễn Thái Ngọc Duy, Jan 17, 2019
  14. 12/76 diff.c: convert -u|-p|--patchNguyễn Thái Ngọc Duy, Jan 17, 2019
  15. 13/76 diff.c: convert -U|--unifiedNguyễn Thái Ngọc Duy, Jan 17, 2019
  16. 14/76 diff.c: convert -W|--[no-]function-contextNguyễn Thái Ngọc Duy, Jan 17, 2019
  17. 15/76 diff.c: convert --rawNguyễn Thái Ngọc Duy, Jan 17, 2019
  18. 16/76 diff.c: convert --patch-with-rawNguyễn Thái Ngọc Duy, Jan 17, 2019
  19. 17/76 diff.c: convert --numstat and --shortstatNguyễn Thái Ngọc Duy, Jan 17, 2019
  20. 18/76 diff.c: convert --dirstat and friendsNguyễn Thái Ngọc Duy, Jan 17, 2019
  21. 19/76 diff.c: convert --checkNguyễn Thái Ngọc Duy, Jan 17, 2019
  22. 20/76 diff.c: convert --summaryNguyễn Thái Ngọc Duy, Jan 17, 2019
  23. 21/76 diff.c: convert --patch-with-statNguyễn Thái Ngọc Duy, Jan 17, 2019
  24. 22/76 diff.c: convert --name-onlyNguyễn Thái Ngọc Duy, Jan 17, 2019
  25. 23/76 diff.c: convert --name-statusNguyễn Thái Ngọc Duy, Jan 17, 2019
  26. 24/76 diff.c: convert -s|--no-patchNguyễn Thái Ngọc Duy, Jan 17, 2019
  27. 25/76 diff.c: convert --stat*Nguyễn Thái Ngọc Duy, Jan 17, 2019
  28. SZEDER GáborJan 19, 2019
  29. 26/76 diff.c: convert --[no-]compact-summaryNguyễn Thái Ngọc Duy, Jan 17, 2019
  30. 27/76 diff.c: convert --output-*Nguyễn Thái Ngọc Duy, Jan 17, 2019
  31. 28/76 diff.c: convert -B|--break-rewritesNguyễn Thái Ngọc Duy, Jan 17, 2019
  32. Johannes SchindelinJan 21, 2019
  33. 29/76 diff.c: convert -M|--find-renamesNguyễn Thái Ngọc Duy, Jan 17, 2019
  34. 30/76 diff.c: convert -D|--irreversible-deleteNguyễn Thái Ngọc Duy, Jan 17, 2019
  35. 31/76 diff.c: convert -C|--find-copiesNguyễn Thái Ngọc Duy, Jan 17, 2019
  36. 32/76 diff.c: convert --find-copies-harderNguyễn Thái Ngọc Duy, Jan 17, 2019
  37. 33/76 diff.c: convert --no-renames|--[no--rename-emptyNguyễn Thái Ngọc Duy, Jan 17, 2019
  38. 34/76 diff.c: convert --relativeNguyễn Thái Ngọc Duy, Jan 17, 2019
  39. 35/76 diff.c: convert --[no-]minimalNguyễn Thái Ngọc Duy, Jan 17, 2019
  40. 36/76 diff.c: convert --ignore-some-changesNguyễn Thái Ngọc Duy, Jan 17, 2019
  41. 37/76 diff.c: convert --[no-]indent-heuristicNguyễn Thái Ngọc Duy, Jan 17, 2019
  42. 38/76 diff.c: convert --patienceNguyễn Thái Ngọc Duy, Jan 17, 2019
  43. 39/76 diff.c: convert --histogramNguyễn Thái Ngọc Duy, Jan 17, 2019
  44. 40/76 diff.c: convert --diff-algorithmNguyễn Thái Ngọc Duy, Jan 17, 2019
  45. 41/76 diff.c: convert --anchoredNguyễn Thái Ngọc Duy, Jan 17, 2019
  46. 42/76 diff.c: convert --binaryNguyễn Thái Ngọc Duy, Jan 17, 2019
  47. 43/76 diff.c: convert --full-indexNguyễn Thái Ngọc Duy, Jan 17, 2019
  48. 44/76 diff.c: convert -a|--textNguyễn Thái Ngọc Duy, Jan 17, 2019
  49. 45/76 diff.c: convert -RNguyễn Thái Ngọc Duy, Jan 17, 2019
  50. 46/76 diff.c: convert --[no-]followNguyễn Thái Ngọc Duy, Jan 17, 2019
  51. 47/76 diff.c: convert --[no-]colorNguyễn Thái Ngọc Duy, Jan 17, 2019
  52. 48/76 diff.c: convert --word-diffNguyễn Thái Ngọc Duy, Jan 17, 2019
  53. 49/76 diff.c: convert --word-diff-regexNguyễn Thái Ngọc Duy, Jan 17, 2019
  54. 50/76 diff.c: convert --color-wordsNguyễn Thái Ngọc Duy, Jan 17, 2019
  55. 51/76 diff.c: convert --exit-codeNguyễn Thái Ngọc Duy, Jan 17, 2019
  56. 52/76 diff.c: convert --quietNguyễn Thái Ngọc Duy, Jan 17, 2019
  57. 53/76 diff.c: convert --ext-diffNguyễn Thái Ngọc Duy, Jan 17, 2019
  58. 54/76 diff.c: convert --textconvNguyễn Thái Ngọc Duy, Jan 17, 2019
  59. 55/76 diff.c: convert --ignore-submodulesNguyễn Thái Ngọc Duy, Jan 17, 2019
  60. 56/76 diff.c: convert --submoduleNguyễn Thái Ngọc Duy, Jan 17, 2019
  61. 57/76 diff.c: convert --ws-error-highlightNguyễn Thái Ngọc Duy, Jan 17, 2019
  62. 58/76 diff.c: convert --ita-[in]visible-in-indexNguyễn Thái Ngọc Duy, Jan 17, 2019
  63. 59/76 diff.c: convert -zNguyễn Thái Ngọc Duy, Jan 17, 2019
  64. 60/76 diff.c: convert -lNguyễn Thái Ngọc Duy, Jan 17, 2019
  65. 61/76 diff.c: convert -S|-GNguyễn Thái Ngọc Duy, Jan 17, 2019
  66. 62/76 diff.c: convert --pickaxe-all|--pickaxe-regexNguyễn Thái Ngọc Duy, Jan 17, 2019
  67. 63/76 diff.c: convert -ONguyễn Thái Ngọc Duy, Jan 17, 2019
  68. Johannes SchindelinJan 21, 2019
  69. 64/76 diff.c: convert --find-objectNguyễn Thái Ngọc Duy, Jan 17, 2019
  70. 65/76 diff.c: convert --diff-filterNguyễn Thái Ngọc Duy, Jan 17, 2019
  71. 66/76 diff.c: convert --[no-]abbrevNguyễn Thái Ngọc Duy, Jan 17, 2019
  72. 67/76 diff.c: convert --[src|dst]-prefixNguyễn Thái Ngọc Duy, Jan 17, 2019
  73. 68/76 diff.c: convert --line-prefixNguyễn Thái Ngọc Duy, Jan 17, 2019
  74. 69/76 diff.c: convert --no-prefixNguyễn Thái Ngọc Duy, Jan 17, 2019
  75. 70/76 diff.c: convert --inter-hunk-contextNguyễn Thái Ngọc Duy, Jan 17, 2019
  76. SZEDER GáborJan 19, 2019
  77. 71/76 diff.c: convert --color-movedNguyễn Thái Ngọc Duy, Jan 17, 2019
  78. 72/76 diff.c: convert --color-moved-wsNguyễn Thái Ngọc Duy, Jan 17, 2019
  79. 73/76 diff.c: allow --no-color-moved-wsNguyễn Thái Ngọc Duy, Jan 17, 2019
  80. 74/76 range-diff: use parse_options() instead of diff_opt_parse()Nguyễn Thái Ngọc Duy, Jan 17, 2019
  81. Stefan BellerJan 17, 2019
  82. Duy NguyenJan 18, 2019
  83. 75/76 diff --no-index: use parse_options() instead of diff_opt_parse()Nguyễn Thái Ngọc Duy, Jan 17, 2019
  84. 76/76 am: avoid diff_opt_parse()Nguyễn Thái Ngọc Duy, Jan 17, 2019
  85. Johannes SchindelinJan 17, 2019
  86. Duy NguyenJan 18, 2019
  87. Ævar Arnfjörð BjarmasonJan 17, 2019
  88. Stefan BellerJan 17, 2019

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.