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>

diff --git a/builtin/merge.c b/builtin/merge.c
index 07839b0bb8f922e5bab1757c8739b2c20082a874..de64d7850e99ef5ab7435cecf545b2872e7c77f1 100644 (file)
--- 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 727a8118b8895381e12f7bcc61d7feaf611aad04..21c84e559065213fe3ae9fa88533bb7e72432dec 100644 (file)
--- 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 e05bcea8096b3e3c507126d19e4a535d316fc179..ec01ef722bdc951bb038653de8b24d662efe9d38 100644 (file)
--- 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 37a56d079a36f0dfbafadd3f8c35d7c86371c7bd..50c340474c7664f979a81811f7e22585727e7b4b 100644 (file)
--- 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,
@@ -590,22 +594,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;
 
@@ -617,6 +627,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;
@@ -635,12 +651,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 f1f246387cae57748a04efa139da7a1d4b8951be..4e49185027a049e073c6991ffda7f2ae2a6c487e 100644 (file)
--- 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
 };