assert NOARG/NONEG behavior of parse-options callbacks

When we define a parse-options callback, the flags we put in the option
struct must match what the callback expects. For example, a callback
which does not handle the "unset" parameter should only be used with
PARSE_OPT_NONEG. But since the callback and the option struct are not
defined next to each other, it's easy to get this wrong (as earlier
patches in this series show).

Fortunately, the compiler can help us here: compiling with
-Wunused-parameters can show us which callbacks ignore their "unset"
parameters (and likewise, ones that ignore "arg" expect to be triggered
with PARSE_OPT_NOARG).

But after we've inspected a callback and determined that all of its
callers use the right flags, what do we do next? We'd like to silence
the compiler warning, but do so in a way that will catch any wrong calls
in the future.

We can do that by actually checking those variables and asserting that
they match our expectations. Because this is such a common pattern,
we'll introduce some helper macros. The resulting messages aren't
as descriptive as we could make them, but the file/line information from
BUG() is enough to identify the problem (and anyway, the point is that
these should never be seen).

Each of the annotated callbacks in this patch triggers
-Wunused-parameters, and was manually inspected to make sure all callers
use the correct options (so none of these BUGs should be triggerable).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/apply.c b/apply.c
index c4c9f849b1bae6b4c29d01ac1162a0b47ba3cb9b..3746310cefae7ff8127cc033e0c11b3eb43eecd0 100644 (file)
--- a/apply.c
+++ b/apply.c
@@ -4772,6 +4772,9 @@ static int apply_option_parse_exclude(const struct option *opt,
                                      const char *arg, int unset)
 {
        struct apply_state *state = opt->value;
+
+       BUG_ON_OPT_NEG(unset);
+
        add_name_limit(state, arg, 1);
        return 0;
 }
@@ -4780,6 +4783,9 @@ static int apply_option_parse_include(const struct option *opt,
                                      const char *arg, int unset)
 {
        struct apply_state *state = opt->value;
+
+       BUG_ON_OPT_NEG(unset);
+
        add_name_limit(state, arg, 0);
        state->has_include = 1;
        return 0;
@@ -4790,6 +4796,9 @@ static int apply_option_parse_p(const struct option *opt,
                                int unset)
 {
        struct apply_state *state = opt->value;
+
+       BUG_ON_OPT_NEG(unset);
+
        state->p_value = atoi(arg);
        state->p_value_known = 1;
        return 0;
@@ -4799,6 +4808,9 @@ static int apply_option_parse_space_change(const struct option *opt,
                                           const char *arg, int unset)
 {
        struct apply_state *state = opt->value;
+
+       BUG_ON_OPT_ARG(arg);
+
        if (unset)
                state->ws_ignore_action = ignore_ws_none;
        else
@@ -4810,6 +4822,9 @@ static int apply_option_parse_whitespace(const struct option *opt,
                                         const char *arg, int unset)
 {
        struct apply_state *state = opt->value;
+
+       BUG_ON_OPT_NEG(unset);
+
        state->whitespace_option = arg;
        if (parse_whitespace_option(state, arg))
                return -1;
@@ -4820,6 +4835,9 @@ static int apply_option_parse_directory(const struct option *opt,
                                        const char *arg, int unset)
 {
        struct apply_state *state = opt->value;
+
+       BUG_ON_OPT_NEG(unset);
+
        strbuf_reset(&state->root);
        strbuf_addstr(&state->root, arg);
        strbuf_complete(&state->root, '/');

diff --git a/builtin/blame.c b/builtin/blame.c
index a443af9ee9dad5ec20de7e75640947c221e7aa6a..06a7163ffe269cd43dc783c81ca6ac71b91b3614 100644 (file)
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -732,6 +732,8 @@ static int blame_copy_callback(const struct option *option, const char *arg, int
 {
        int *opt = option->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        /*
         * -C enables copy from removed files;
         * -C -C enables copy from existing files, but only
@@ -754,6 +756,8 @@ static int blame_move_callback(const struct option *option, const char *arg, int
 {
        int *opt = option->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        *opt |= PICKAXE_BLAME_MOVE;
 
        if (arg)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0f6b692df6bfc5704fd853e7d5ee6222a352d71e..1cdd357d523eaaba6c86a78ac21cc068e87d371b 100644 (file)
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -595,6 +595,8 @@ static int batch_option_callback(const struct option *opt,
 {
        struct batch_options *bo = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (bo->enabled) {
                return error(_("only one batch option may be specified"));
        }

diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 88b86c8d9f5a0ea9c49b42e891082203b80a977f..eb74774cbc9d6bd6acf1b7fe2f31b8e80e461a12 100644 (file)
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -132,6 +132,8 @@ static const char * const builtin_checkout_index_usage[] = {
 static int option_parse_stage(const struct option *opt,
                              const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
+
        if (!strcmp(arg, "all")) {
                to_tempfile = 1;
                checkout_stage = CHECKOUT_ALL;

diff --git a/builtin/clean.c b/builtin/clean.c
index 8d9a7dc20647079bc5f35b446bbbab5479abb32d..bbcdeb2d9e7f275309609a2a21a5dd4baf1cf1bd 100644 (file)
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -140,6 +140,7 @@ static void clean_print_color(enum color_clean ix)
 static int exclude_cb(const struct option *opt, const char *arg, int unset)
 {
        struct string_list *exclude_list = opt->value;
+       BUG_ON_OPT_NEG(unset);
        string_list_append(exclude_list, arg);
        return 0;
 }

diff --git a/builtin/commit.c b/builtin/commit.c
index 4d7754ca43515575730b10c6c40bbc6234424e06..500f793fa6947f6309ce4ea82d15410e35b2ff20 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -161,6 +161,9 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 static int opt_parse_rename_score(const struct option *opt, const char *arg, int unset)
 {
        const char **value = opt->value;
+
+       BUG_ON_OPT_NEG(unset);
+
        if (arg != NULL && *arg == '=')
                arg = arg + 1;
 

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8f7249f2b138279dabfe994248f58069e8d52a7b..354ed8bdef3a6511ddb580d97b75df60b77ac6ca 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -98,6 +98,8 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
 
 static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
+
        /*
         * "git fetch --refmap='' origin foo"
         * can be used to tell the command not to store anywhere

diff --git a/builtin/grep.c b/builtin/grep.c
index d8508ddf792dd22dc0c8c283df29e5e6d9dacbf7..33c8b61595652889f15bef21bb4f7d1af6429170 100644 (file)
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -712,11 +712,14 @@ static int context_callback(const struct option *opt, const char *arg,
 static int file_callback(const struct option *opt, const char *arg, int unset)
 {
        struct grep_opt *grep_opt = opt->value;
-       int from_stdin = !strcmp(arg, "-");
+       int from_stdin;
        FILE *patterns;
        int lno = 0;
        struct strbuf sb = STRBUF_INIT;
 
+       BUG_ON_OPT_NEG(unset);
+
+       from_stdin = !strcmp(arg, "-");
        patterns = from_stdin ? stdin : fopen(arg, "r");
        if (!patterns)
                die_errno(_("cannot open '%s'"), arg);
@@ -737,6 +740,8 @@ static int file_callback(const struct option *opt, const char *arg, int unset)
 static int not_callback(const struct option *opt, const char *arg, int unset)
 {
        struct grep_opt *grep_opt = opt->value;
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        append_grep_pattern(grep_opt, "--not", "command line", 0, GREP_NOT);
        return 0;
 }
@@ -744,6 +749,8 @@ static int not_callback(const struct option *opt, const char *arg, int unset)
 static int and_callback(const struct option *opt, const char *arg, int unset)
 {
        struct grep_opt *grep_opt = opt->value;
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        append_grep_pattern(grep_opt, "--and", "command line", 0, GREP_AND);
        return 0;
 }
@@ -751,6 +758,8 @@ static int and_callback(const struct option *opt, const char *arg, int unset)
 static int open_callback(const struct option *opt, const char *arg, int unset)
 {
        struct grep_opt *grep_opt = opt->value;
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        append_grep_pattern(grep_opt, "(", "command line", 0, GREP_OPEN_PAREN);
        return 0;
 }
@@ -758,6 +767,8 @@ static int open_callback(const struct option *opt, const char *arg, int unset)
 static int close_callback(const struct option *opt, const char *arg, int unset)
 {
        struct grep_opt *grep_opt = opt->value;
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        append_grep_pattern(grep_opt, ")", "command line", 0, GREP_CLOSE_PAREN);
        return 0;
 }
@@ -766,6 +777,7 @@ static int pattern_callback(const struct option *opt, const char *arg,
                            int unset)
 {
        struct grep_opt *grep_opt = opt->value;
+       BUG_ON_OPT_NEG(unset);
        append_grep_pattern(grep_opt, arg, "-e option", 0, GREP_PATTERN);
        return 0;
 }

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 12ddda7e7bac51f74b2f7ca8a2b64ffe90fd8683..41faffd28db8850fb97daa52c6d481375b380504 100644 (file)
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -451,6 +451,7 @@ static int guess_repository_type(const char *git_dir)
 
 static int shared_callback(const struct option *opt, const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
        *((int *) opt->value) = (arg) ? git_config_perm("arg", arg) : PERM_GROUP;
        return 0;
 }

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 4b87e0dd2e8854bf940e92e3020c5e4f66e8a6bd..8ae40dec4746571cf80d2f76bbad067c33a972fa 100644 (file)
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -80,6 +80,8 @@ static int parse_opt_parse(const struct option *opt, const char *arg,
        v->only_trailers = 1;
        v->only_input = 1;
        v->unfold = 1;
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        return 0;
 }
 

diff --git a/builtin/log.c b/builtin/log.c
index 41188e723cc1f259caf5b0c4399cd9745455f1b1..9f2d9872946dfd7fe527a7f3822d9be23aa5ec83 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -107,6 +107,8 @@ static int log_line_range_callback(const struct option *option, const char *arg,
 {
        struct line_opt_callback_data *data = option->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (!arg)
                return -1;
 
@@ -1151,6 +1153,8 @@ static int keep_subject = 0;
 
 static int keep_callback(const struct option *opt, const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        ((struct rev_info *)opt->value)->total = -1;
        keep_subject = 1;
        return 0;
@@ -1161,6 +1165,7 @@ static int subject_prefix = 0;
 static int subject_prefix_callback(const struct option *opt, const char *arg,
                            int unset)
 {
+       BUG_ON_OPT_NEG(unset);
        subject_prefix = 1;
        ((struct rev_info *)opt->value)->subject_prefix = arg;
        return 0;
@@ -1168,6 +1173,8 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,
 
 static int rfc_callback(const struct option *opt, const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        return subject_prefix_callback(opt, "RFC PATCH", unset);
 }
 
@@ -1176,6 +1183,7 @@ static int numbered_cmdline_opt = 0;
 static int numbered_callback(const struct option *opt, const char *arg,
                             int unset)
 {
+       BUG_ON_OPT_ARG(arg);
        *(int *)opt->value = numbered_cmdline_opt = unset ? 0 : 1;
        if (unset)
                auto_number =  0;
@@ -1185,6 +1193,7 @@ static int numbered_callback(const struct option *opt, const char *arg,
 static int no_numbered_callback(const struct option *opt, const char *arg,
                                int unset)
 {
+       BUG_ON_OPT_NEG(unset);
        return numbered_callback(opt, arg, 1);
 }
 
@@ -1192,6 +1201,7 @@ static int output_directory_callback(const struct option *opt, const char *arg,
                              int unset)
 {
        const char **dir = (const char **)opt->value;
+       BUG_ON_OPT_NEG(unset);
        if (*dir)
                die(_("Two output directories?"));
        *dir = arg;

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 2787453eb1df713ecf9c2a0d3a5ee25659aac557..c70a9c71588a76b2ef04689fc6cd805cee4ae0b8 100644 (file)
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -475,6 +475,8 @@ static int option_parse_exclude(const struct option *opt,
 {
        struct string_list *exclude_list = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        exc_given = 1;
        string_list_append(exclude_list, arg);
 
@@ -486,6 +488,8 @@ static int option_parse_exclude_from(const struct option *opt,
 {
        struct dir_struct *dir = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        exc_given = 1;
        add_excludes_from_file(dir, arg);
 
@@ -497,6 +501,9 @@ static int option_parse_exclude_standard(const struct option *opt,
 {
        struct dir_struct *dir = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
+
        exc_given = 1;
        setup_standard_excludes(dir);
 

diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index b08803e61119a569e2ca5a33402666d8978d9ff0..06a2f90c4875f2a8ff82a927071ac584ad929f8b 100644 (file)
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -15,6 +15,8 @@ static int label_cb(const struct option *opt, const char *arg, int unset)
        static int label_count = 0;
        const char **names = (const char **)opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (label_count >= 3)
                return error("too many labels on the command line");
        names[label_count++] = arg;

diff --git a/builtin/merge.c b/builtin/merge.c
index 4aa60715980f2f226d752e9d7fe0df4ecdcc369d..adb0402e847a50bc9e9be69e2257b1b555a780ec 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -224,6 +224,7 @@ static int option_parse_x(const struct option *opt,
 static int option_parse_n(const struct option *opt,
                          const char *arg, int unset)
 {
+       BUG_ON_OPT_ARG(arg);
        show_diffstat = unset;
        return 0;
 }

diff --git a/builtin/notes.c b/builtin/notes.c
index c05cd004abcbdcd1e4c3d140f3cf4251c416df36..91faa514aaa7f83b06f8ffc9599164c366c394fb 100644 (file)
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -215,6 +215,8 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 {
        struct note_data *d = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        strbuf_grow(&d->buf, strlen(arg) + 2);
        if (d->buf.len)
                strbuf_addch(&d->buf, '\n');
@@ -229,6 +231,8 @@ static int parse_file_arg(const struct option *opt, const char *arg, int unset)
 {
        struct note_data *d = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (d->buf.len)
                strbuf_addch(&d->buf, '\n');
        if (!strcmp(arg, "-")) {
@@ -250,6 +254,8 @@ static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
        enum object_type type;
        unsigned long len;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (d->buf.len)
                strbuf_addch(&d->buf, '\n');
 
@@ -273,6 +279,7 @@ static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
 static int parse_reedit_arg(const struct option *opt, const char *arg, int unset)
 {
        struct note_data *d = opt->value;
+       BUG_ON_OPT_NEG(unset);
        d->use_editor = 1;
        return parse_reuse_arg(opt, arg, unset);
 }

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4eac2f997bb0334ff6be93d292b00b35c5e28c63..6718e62c15ded347f122a985711a965d74bdf7b8 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3206,6 +3206,9 @@ static int option_parse_index_version(const struct option *opt,
 {
        char *c;
        const char *val = arg;
+
+       BUG_ON_OPT_NEG(unset);
+
        pack_idx_opts.version = strtoul(val, &c, 10);
        if (pack_idx_opts.version > 2)
                die(_("unsupported index version %s"), val);

diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index fbbc98e5161f011a25743a528e6bb3f85e415f51..183ee8c1e5d4de53d10e9ed00c624f615baf3fc9 100644 (file)
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -44,6 +44,7 @@ static const char * const read_tree_usage[] = {
 static int index_output_cb(const struct option *opt, const char *arg,
                                 int unset)
 {
+       BUG_ON_OPT_NEG(unset);
        set_alternate_index_output(arg);
        return 0;
 }
@@ -54,6 +55,8 @@ static int exclude_per_directory_cb(const struct option *opt, const char *arg,
        struct dir_struct *dir;
        struct unpack_trees_options *opts;
 
+       BUG_ON_OPT_NEG(unset);
+
        opts = (struct unpack_trees_options *)opt->value;
 
        if (opts->dir)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa3631ee89761d570b433994cc77302742e..b84568fc4e6677e5f68c3b11e0798942ae19577a 100644 (file)
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -703,6 +703,9 @@ static int parse_opt_merge(const struct option *opt, const char *arg, int unset)
 {
        struct rebase_options *opts = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
+
        if (!is_interactive(opts))
                opts->type = REBASE_MERGE;
 
@@ -715,6 +718,9 @@ static int parse_opt_interactive(const struct option *opt, const char *arg,
 {
        struct rebase_options *opts = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
+
        opts->type = REBASE_INTERACTIVE;
        opts->flags |= REBASE_INTERACTIVE_EXPLICIT;
 

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index b1b540f7f6cf90a8cec3816ffa4404449b9e5bb4..934e5149448783e32acc8b605f202384af7d96ee 100644 (file)
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -604,6 +604,7 @@ static int parse_reflog_param(const struct option *opt, const char *arg,
 {
        char *ep;
        const char **base = (const char **)opt->value;
+       BUG_ON_OPT_NEG(unset);
        if (!arg)
                arg = "";
        reflog = strtoul(arg, &ep, 10);

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 2f13f1316fadc2fc860fac75a161b47f7d11e391..ed888ffa48ce1d5ef80859b8f5b3bf1720e49abd 100644 (file)
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -151,6 +151,7 @@ static int hash_callback(const struct option *opt, const char *arg, int unset)
 static int exclude_existing_callback(const struct option *opt, const char *arg,
                                     int unset)
 {
+       BUG_ON_OPT_NEG(unset);
        exclude_arg = 1;
        *(const char **)opt->value = arg;
        return 0;

diff --git a/builtin/tag.c b/builtin/tag.c
index 6a396a5090f1e6cf0ed73f001a6bef006789b26f..02f6bd1279d9c360f97d04ebba0b1f88ff2f27e1 100644 (file)
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -338,6 +338,8 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 {
        struct msg_arg *msg = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (!arg)
                return -1;
        if (msg->buf.len)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 07c10bcb7d82c19becf424f9ab63954c9e58f93f..faa16c61f10629866982ecce9f9dc9224bd79216 100644 (file)
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -790,12 +790,16 @@ static int refresh(struct refresh_params *o, unsigned int flag)
 static int refresh_callback(const struct option *opt,
                                const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        return refresh(opt->value, 0);
 }
 
 static int really_refresh_callback(const struct option *opt,
                                const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        return refresh(opt->value, REFRESH_REALLY);
 }
 
@@ -803,6 +807,7 @@ static int chmod_callback(const struct option *opt,
                                const char *arg, int unset)
 {
        char *flip = opt->value;
+       BUG_ON_OPT_NEG(unset);
        if ((arg[0] != '-' && arg[0] != '+') || arg[1] != 'x' || arg[2])
                return error("option 'chmod' expects \"+x\" or \"-x\"");
        *flip = arg[0];
@@ -812,6 +817,8 @@ static int chmod_callback(const struct option *opt,
 static int resolve_undo_clear_callback(const struct option *opt,
                                const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
        resolve_undo_clear();
        return 0;
 }
@@ -847,6 +854,8 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
        unsigned int mode;
        const char *path;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (!parse_new_style_cacheinfo(ctx->argv[1], &mode, &oid, &path)) {
                if (add_cacheinfo(mode, &oid, path, 0))
                        die("git update-index: --cacheinfo cannot add %s", path);
@@ -869,6 +878,8 @@ static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
 {
        int *nul_term_line = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (ctx->argc != 1)
                return error("option '%s' must be the last argument", opt->long_name);
        allow_add = allow_replace = allow_remove = 1;
@@ -881,6 +892,8 @@ static int stdin_callback(struct parse_opt_ctx_t *ctx,
 {
        int *read_from_stdin = opt->value;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (ctx->argc != 1)
                return error("option '%s' must be the last argument", opt->long_name);
        *read_from_stdin = 1;
@@ -888,11 +901,13 @@ static int stdin_callback(struct parse_opt_ctx_t *ctx,
 }
 
 static int unresolve_callback(struct parse_opt_ctx_t *ctx,
-                               const struct option *opt, int flags)
+                               const struct option *opt, int unset)
 {
        int *has_errors = opt->value;
        const char *prefix = startup_info->prefix;
 
+       BUG_ON_OPT_NEG(unset);
+
        /* consume remaining arguments. */
        *has_errors = do_unresolve(ctx->argc, ctx->argv,
                                prefix, prefix ? strlen(prefix) : 0);
@@ -905,11 +920,13 @@ static int unresolve_callback(struct parse_opt_ctx_t *ctx,
 }
 
 static int reupdate_callback(struct parse_opt_ctx_t *ctx,
-                               const struct option *opt, int flags)
+                               const struct option *opt, int unset)
 {
        int *has_errors = opt->value;
        const char *prefix = startup_info->prefix;
 
+       BUG_ON_OPT_NEG(unset);
+
        /* consume remaining arguments. */
        setup_work_tree();
        *has_errors = do_reupdate(ctx->argc, ctx->argv,

diff --git a/parse-options-cb.c b/parse-options-cb.c
index 6a61166b265caae09564586017460797ee928a43..8c9edce52f63bcb1085b119b3a2264a97b1fb374 100644 (file)
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -58,6 +58,8 @@ int parse_opt_verbosity_cb(const struct option *opt, const char *arg,
 {
        int *target = opt->value;
 
+       BUG_ON_OPT_ARG(arg);
+
        if (unset)
                /* --no-quiet, --no-verbose */
                *target = 0;
@@ -80,6 +82,8 @@ int parse_opt_commits(const struct option *opt, const char *arg, int unset)
        struct object_id oid;
        struct commit *commit;
 
+       BUG_ON_OPT_NEG(unset);
+
        if (!arg)
                return -1;
        if (get_oid(arg, &oid))
@@ -110,6 +114,9 @@ int parse_opt_object_name(const struct option *opt, const char *arg, int unset)
 int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
 {
        int *target = opt->value;
+
+       BUG_ON_OPT_ARG(arg);
+
        *target = unset ? 2 : 1;
        return 0;
 }

diff --git a/parse-options.h b/parse-options.h
index c3f2d2eceb5ade24afc642167a635f52ea56a37c..6c4fe2016d65f1093400f8f5080085eb244f0026 100644 (file)
--- a/parse-options.h
+++ b/parse-options.h
@@ -191,6 +191,20 @@ extern int opterror(const struct option *opt, const char *reason, int flags);
 #define opterror(o,r,f) (opterror((o),(r),(f)), const_error())
 #endif
 
+/*
+ * Use these assertions for callbacks that expect to be called with NONEG and
+ * NOARG respectively, and do not otherwise handle the "unset" and "arg"
+ * parameters.
+ */
+#define BUG_ON_OPT_NEG(unset) do { \
+       if ((unset)) \
+               BUG("option callback does not expect negation"); \
+} while (0)
+#define BUG_ON_OPT_ARG(arg) do { \
+       if ((arg)) \
+               BUG("option callback does not expect an argument"); \
+} while (0)
+
 /*----- incremental advanced APIs -----*/
 
 enum {

diff --git a/ref-filter.c b/ref-filter.c
index 0c45ed9d94a4bd4bfab6c9d441ca9bc7de7ebf31..7eca4362236cb6aee15999ee290a08c4ee06278d 100644 (file)
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2316,6 +2316,8 @@ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
        struct object_id oid;
        int no_merged = starts_with(opt->long_name, "no");
 
+       BUG_ON_OPT_NEG(unset);
+
        if (rf->merge) {
                if (no_merged) {
                        return opterror(opt, "is incompatible with --merged", 0);

diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index f0623bb42bb1917a481c676316fe609779f84323..47fee660b86a6e6977735e2c940de79d4330d519 100644 (file)
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -36,6 +36,7 @@ static int length_callback(const struct option *opt, const char *arg, int unset)
 
 static int number_callback(const struct option *opt, const char *arg, int unset)
 {
+       BUG_ON_OPT_NEG(unset);
        *(int *)opt->value = strtol(arg, NULL, 10);
        return 0;
 }