From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Stefan Beller" <sbeller@google.com>,
"Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH 06/14] parse-options: avoid magic return codes
Date: Sun, 27 Jan 2019 07:35:27 +0700 [thread overview]
Message-ID: <20190127003535.28341-7-pclouds@gmail.com> (raw)
In-Reply-To: <20190127003535.28341-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>
Signed-off-by: Junio C Hamano <gitster@pobox.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 37a56d079a..50c340474c 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,
@@ -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 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.1.560.g70ca8b83ee
next prev parent reply other threads:[~2019-01-27 0:36 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-27 0:35 [PATCH 00/14] nd/diff-parseopt part 1 Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 01/14] parse-options.h: remove extern on function prototypes Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 02/14] parse-options: add one-shot mode Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 03/14] parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 04/14] parse-options: add OPT_BITOP() Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 05/14] parse-options: stop abusing 'callback' for lowlevel callbacks Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` Nguyễn Thái Ngọc Duy [this message]
2019-01-27 0:35 ` [PATCH 07/14] parse-options: allow ll_callback with OPTION_CALLBACK Nguyễn Thái Ngọc Duy
2019-04-15 14:06 ` Derrick Stolee
2019-04-16 8:52 ` Duy Nguyen
2019-04-16 14:24 ` Derrick Stolee
2019-01-27 0:35 ` [PATCH 08/14] diff.h: keep forward struct declarations sorted Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 09/14] diff.h: avoid bit fields in struct diff_flags Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 10/14] diff.c: prepare to use parse_options() for parsing Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 11/14] diff.c: convert -u|-p|--patch Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 12/14] diff.c: convert -U|--unified Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 13/14] diff.c: convert -W|--[no-]function-context Nguyễn Thái Ngọc Duy
2019-01-27 0:35 ` [PATCH 14/14] diff.c: convert --raw Nguyễn Thái Ngọc Duy
2019-01-28 0:33 ` [PATCH 00/14] nd/diff-parseopt part 1 Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190127003535.28341-7-pclouds@gmail.com \
--to=pclouds@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.