parse-options: avoid magic return codes

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Jan 27, 2019 at 07:35 UTC f41179f16ba2fc16e31be81518536008afe2e278
5 files changed +68 -46
builtin/merge.c
+3 -2
@@ -112,8 +112,9 @@ static int option_parse_message(const struct option *opt,
112 return 0;
113 }
114
115 -static int option_read_message(struct parse_opt_ctx_t *ctx,
116 - const struct option *opt, int unset)
115 +static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
116 + const struct option *opt,
117 + int unset)
118 {
119 struct strbuf *buf = opt->value;
120 const char *arg;
builtin/update-index.c
+10 -10
@@ -847,8 +847,8 @@ static int parse_new_style_cacheinfo(const char *arg,
847 return 0;
848 }
849
850 -static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
851 - const struct option *opt, int unset)
850 +static enum parse_opt_result cacheinfo_callback(
851 + struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
852 {
853 struct object_id oid;
854 unsigned int mode;
@@ -873,8 +873,8 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
873 return 0;
874 }
875
876 -static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
877 - const struct option *opt, int unset)
876 +static enum parse_opt_result stdin_cacheinfo_callback(
877 + struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
878 {
879 int *nul_term_line = opt->value;
880
@@ -887,8 +887,8 @@ static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
887 return 0;
888 }
889
890 -static int stdin_callback(struct parse_opt_ctx_t *ctx,
891 - const struct option *opt, int unset)
890 +static enum parse_opt_result stdin_callback(
891 + struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
892 {
893 int *read_from_stdin = opt->value;
894
@@ -900,8 +900,8 @@ static int stdin_callback(struct parse_opt_ctx_t *ctx,
900 return 0;
901 }
902
903 -static int unresolve_callback(struct parse_opt_ctx_t *ctx,
904 - const struct option *opt, int unset)
903 +static enum parse_opt_result unresolve_callback(
904 + struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
905 {
906 int *has_errors = opt->value;
907 const char *prefix = startup_info->prefix;
@@ -919,8 +919,8 @@ static int unresolve_callback(struct parse_opt_ctx_t *ctx,
919 return 0;
920 }
921
922 -static int reupdate_callback(struct parse_opt_ctx_t *ctx,
923 - const struct option *opt, int unset)
922 +static enum parse_opt_result reupdate_callback(
923 + struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
924 {
925 int *has_errors = opt->value;
926 const char *prefix = startup_info->prefix;
parse-options-cb.c
+3 -3
@@ -170,10 +170,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
170 * "-h" output even if it's not being handled directly by
171 * parse_options().
172 */
173 -int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
174 - const struct option *opt, int unset)
173 +enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
174 + const struct option *opt, int unset)
175 {
176 - return -2;
176 + return PARSE_OPT_UNKNOWN;
177 }
178
179 /**
parse-options.c
+45 -24
@@ -20,8 +20,9 @@ int optbug(const struct option *opt, const char *reason)
20 return error("BUG: switch '%c' %s", opt->short_name, reason);
21 }
22
23 -static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
24 - int flags, const char **arg)
23 +static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
24 + const struct option *opt,
25 + int flags, const char **arg)
26 {
27 if (p->opt) {
28 *arg = p->opt;
@@ -44,9 +45,10 @@ static void fix_filename(const char *prefix, const char **file)
45 *file = prefix_filename(prefix, *file);
46 }
47
47 -static int opt_command_mode_error(const struct option *opt,
48 - const struct option *all_opts,
49 - int flags)
48 +static enum parse_opt_result opt_command_mode_error(
49 + const struct option *opt,
50 + const struct option *all_opts,
51 + int flags)
52 {
53 const struct option *that;
54 struct strbuf that_name = STRBUF_INIT;
@@ -69,16 +71,16 @@ static int opt_command_mode_error(const struct option *opt,
71 error(_("%s is incompatible with %s"),
72 optname(opt, flags), that_name.buf);
73 strbuf_release(&that_name);
72 - return -1;
74 + return PARSE_OPT_ERROR;
75 }
76 return error(_("%s : incompatible with something else"),
77 optname(opt, flags));
78 }
79
78 -static int get_value(struct parse_opt_ctx_t *p,
79 - const struct option *opt,
80 - const struct option *all_opts,
81 - int flags)
80 +static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
81 + const struct option *opt,
82 + const struct option *all_opts,
83 + int flags)
84 {
85 const char *s, *arg;
86 const int unset = flags & OPT_UNSET;
@@ -208,7 +210,8 @@ static int get_value(struct parse_opt_ctx_t *p,
210 }
211 }
212
211 -static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
213 +static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
214 + const struct option *options)
215 {
216 const struct option *all_opts = options;
217 const struct option *numopt = NULL;
@@ -239,11 +242,12 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
242 free(arg);
243 return rc;
244 }
242 - return -2;
245 + return PARSE_OPT_UNKNOWN;
246 }
247
245 -static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
246 - const struct option *options)
248 +static enum parse_opt_result parse_long_opt(
249 + struct parse_opt_ctx_t *p, const char *arg,
250 + const struct option *options)
251 {
252 const struct option *all_opts = options;
253 const char *arg_end = strchrnul(arg, '=');
@@ -269,7 +273,7 @@ again:
273 if (*rest)
274 continue;
275 p->out[p->cpidx++] = arg - 2;
272 - return 0;
276 + return PARSE_OPT_DONE;
277 }
278 if (!rest) {
279 /* abbreviated? */
@@ -334,11 +338,11 @@ is_abbreviated:
338 ambiguous_option->long_name,
339 (abbrev_flags & OPT_UNSET) ? "no-" : "",
340 abbrev_option->long_name);
337 - return -3;
341 + return PARSE_OPT_HELP;
342 }
343 if (abbrev_option)
344 return get_value(p, abbrev_option, all_opts, abbrev_flags);
341 - return -2;
345 + return PARSE_OPT_UNKNOWN;
346 }
347
348 static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
@@ -590,22 +594,28 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
594 if (arg[1] != '-') {
595 ctx->opt = arg + 1;
596 switch (parse_short_opt(ctx, options)) {
593 - case -1:
597 + case PARSE_OPT_ERROR:
598 return PARSE_OPT_ERROR;
595 - case -2:
599 + case PARSE_OPT_UNKNOWN:
600 if (ctx->opt)
601 check_typos(arg + 1, options);
602 if (internal_help && *ctx->opt == 'h')
603 goto show_usage;
604 goto unknown;
605 + case PARSE_OPT_NON_OPTION:
606 + case PARSE_OPT_HELP:
607 + case PARSE_OPT_COMPLETE:
608 + BUG("parse_short_opt() cannot return these");
609 + case PARSE_OPT_DONE:
610 + break;
611 }
612 if (ctx->opt)
613 check_typos(arg + 1, options);
614 while (ctx->opt) {
615 switch (parse_short_opt(ctx, options)) {
606 - case -1:
616 + case PARSE_OPT_ERROR:
617 return PARSE_OPT_ERROR;
608 - case -2:
618 + case PARSE_OPT_UNKNOWN:
619 if (internal_help && *ctx->opt == 'h')
620 goto show_usage;
621
@@ -617,6 +627,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
627 ctx->argv[0] = xstrdup(ctx->opt - 1);
628 *(char *)ctx->argv[0] = '-';
629 goto unknown;
630 + case PARSE_OPT_NON_OPTION:
631 + case PARSE_OPT_COMPLETE:
632 + case PARSE_OPT_HELP:
633 + BUG("parse_short_opt() cannot return these");
634 + case PARSE_OPT_DONE:
635 + break;
636 }
637 }
638 continue;
@@ -635,12 +651,17 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
651 if (internal_help && !strcmp(arg + 2, "help"))
652 goto show_usage;
653 switch (parse_long_opt(ctx, arg + 2, options)) {
638 - case -1:
654 + case PARSE_OPT_ERROR:
655 return PARSE_OPT_ERROR;
640 - case -2:
656 + case PARSE_OPT_UNKNOWN:
657 goto unknown;
642 - case -3:
658 + case PARSE_OPT_HELP:
659 goto show_usage;
660 + case PARSE_OPT_NON_OPTION:
661 + case PARSE_OPT_COMPLETE:
662 + BUG("parse_long_opt() cannot return these");
663 + case PARSE_OPT_DONE:
664 + break;
665 }
666 continue;
667 unknown:
parse-options.h
+7 -7
@@ -49,8 +49,8 @@ struct option;
49 typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
50
51 struct parse_opt_ctx_t;
52 -typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
53 - const struct option *opt, int unset);
52 +typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
53 + const struct option *opt, int unset);
54
55 /*
56 * `type`::
@@ -222,12 +222,12 @@ const char *optname(const struct option *opt, int flags);
222
223 /*----- incremental advanced APIs -----*/
224
225 -enum {
226 - PARSE_OPT_COMPLETE = -2,
227 - PARSE_OPT_HELP = -1,
228 - PARSE_OPT_DONE,
225 +enum parse_opt_result {
226 + PARSE_OPT_COMPLETE = -3,
227 + PARSE_OPT_HELP = -2,
228 + PARSE_OPT_ERROR = -1, /* must be the same as error() */
229 + PARSE_OPT_DONE = 0, /* fixed so that "return 0" works */
230 PARSE_OPT_NON_OPTION,
230 - PARSE_OPT_ERROR,
231 PARSE_OPT_UNKNOWN
232 };
233