diff options
| author | Junio C Hamano <gitster@pobox.com> | 2025-07-16 09:42:28 -0700 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2025-07-16 09:42:28 -0700 |
| commit | 0fd2a2ec142af06aa46343bcb053206043b14308 (patch) | |
| tree | 4d8f368941adfeab16464534c36b5fcf620d77fc | |
| parent | edb4fd966974c8b64be527e321f2af70a7f89822 (diff) | |
| parent | c1e616c39b31e78acc595790bf3a9553a022a19d (diff) | |
| download | git-0fd2a2ec142af06aa46343bcb053206043b14308.tar.gz | |
Merge branch 'rs/parse-options-precision'
Define .precision to more canned parse-options type to avoid bugs
coming from using a variable with a wrong type to capture the
parsed values.
* rs/parse-options-precision:
parse-options: add precision handling for OPTION_COUNTUP
parse-options: add precision handling for OPTION_BITOP
parse-options: add precision handling for OPTION_NEGBIT
parse-options: add precision handling for OPTION_BIT
parse-options: add precision handling for OPTION_SET_INT
parse-options: add precision handling for PARSE_OPT_CMDMODE
parse-options: require PARSE_OPT_NOARG for OPTION_BITOP
| -rw-r--r-- | builtin/am.c | 1 | ||||
| -rw-r--r-- | builtin/rebase.c | 1 | ||||
| -rw-r--r-- | builtin/update-index.c | 6 | ||||
| -rw-r--r-- | builtin/write-tree.c | 1 | ||||
| -rw-r--r-- | parse-options.c | 155 | ||||
| -rw-r--r-- | parse-options.h | 7 | ||||
| -rw-r--r-- | t/helper/test-parse-options.c | 17 |
7 files changed, 146 insertions, 42 deletions
diff --git a/builtin/am.c b/builtin/am.c index a800003340..c9d925f7b9 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2406,6 +2406,7 @@ int cmd_am(int argc, .type = OPTION_CALLBACK, .long_name = "show-current-patch", .value = &resume_mode, + .precision = sizeof(resume_mode), .argh = "(diff|raw)", .help = N_("show the patch being applied"), .flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP, diff --git a/builtin/rebase.c b/builtin/rebase.c index 2e8c4ee678..e90562a3b8 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1128,6 +1128,7 @@ int cmd_rebase(int argc, .short_name = 'n', .long_name = "no-stat", .value = &options.flags, + .precision = sizeof(options.flags), .help = N_("do not show diffstat of what changed upstream"), .flags = PARSE_OPT_NOARG, .defval = REBASE_DIFFSTAT, diff --git a/builtin/update-index.c b/builtin/update-index.c index 538b619ba4..0c1d4ed55b 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -981,6 +981,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "assume-unchanged", .value = &mark_valid_only, + .precision = sizeof(mark_valid_only), .help = N_("mark files as \"not changing\""), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = MARK_FLAG, @@ -989,6 +990,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "no-assume-unchanged", .value = &mark_valid_only, + .precision = sizeof(mark_valid_only), .help = N_("clear assumed-unchanged bit"), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = UNMARK_FLAG, @@ -997,6 +999,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "skip-worktree", .value = &mark_skip_worktree_only, + .precision = sizeof(mark_skip_worktree_only), .help = N_("mark files as \"index-only\""), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = MARK_FLAG, @@ -1005,6 +1008,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "no-skip-worktree", .value = &mark_skip_worktree_only, + .precision = sizeof(mark_skip_worktree_only), .help = N_("clear skip-worktree bit"), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = UNMARK_FLAG, @@ -1079,6 +1083,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "fsmonitor-valid", .value = &mark_fsmonitor_only, + .precision = sizeof(mark_fsmonitor_only), .help = N_("mark files as fsmonitor valid"), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = MARK_FLAG, @@ -1087,6 +1092,7 @@ int cmd_update_index(int argc, .type = OPTION_SET_INT, .long_name = "no-fsmonitor-valid", .value = &mark_fsmonitor_only, + .precision = sizeof(mark_fsmonitor_only), .help = N_("clear fsmonitor valid bit"), .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = UNMARK_FLAG, diff --git a/builtin/write-tree.c b/builtin/write-tree.c index 5a8dc377ec..cfec044710 100644 --- a/builtin/write-tree.c +++ b/builtin/write-tree.c @@ -35,6 +35,7 @@ int cmd_write_tree(int argc, .type = OPTION_BIT, .long_name = "ignore-cache-tree", .value = &flags, + .precision = sizeof(flags), .help = N_("only useful for debugging"), .flags = PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, .defval = WRITE_TREE_IGNORE_CACHE_TREE, diff --git a/parse-options.c b/parse-options.c index a9a39ecaef..5224203ffe 100644 --- a/parse-options.c +++ b/parse-options.c @@ -68,6 +68,64 @@ static char *fix_filename(const char *prefix, const char *file) return prefix_filename_except_for_dash(prefix, file); } +static int do_get_int_value(const void *value, size_t precision, intmax_t *ret) +{ + switch (precision) { + case sizeof(int8_t): + *ret = *(int8_t *)value; + return 0; + case sizeof(int16_t): + *ret = *(int16_t *)value; + return 0; + case sizeof(int32_t): + *ret = *(int32_t *)value; + return 0; + case sizeof(int64_t): + *ret = *(int64_t *)value; + return 0; + default: + return -1; + } +} + +static intmax_t get_int_value(const struct option *opt, enum opt_parsed flags) +{ + intmax_t ret; + if (do_get_int_value(opt->value, opt->precision, &ret)) + BUG("invalid precision for option %s", optname(opt, flags)); + return ret; +} + +static enum parse_opt_result set_int_value(const struct option *opt, + enum opt_parsed flags, + intmax_t value) +{ + switch (opt->precision) { + case sizeof(int8_t): + *(int8_t *)opt->value = value; + return 0; + case sizeof(int16_t): + *(int16_t *)opt->value = value; + return 0; + case sizeof(int32_t): + *(int32_t *)opt->value = value; + return 0; + case sizeof(int64_t): + *(int64_t *)opt->value = value; + return 0; + default: + BUG("invalid precision for option %s", optname(opt, flags)); + } +} + +static int signed_int_fits(intmax_t value, size_t precision) +{ + size_t bits = precision * CHAR_BIT; + intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits); + intmax_t lower_bound = -upper_bound - 1; + return lower_bound <= value && value <= upper_bound; +} + static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, const struct option *opt, enum opt_parsed flags, @@ -89,35 +147,55 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, return opt->ll_callback(p, opt, NULL, unset); case OPTION_BIT: + { + intmax_t value = get_int_value(opt, flags); if (unset) - *(int *)opt->value &= ~opt->defval; + value &= ~opt->defval; else - *(int *)opt->value |= opt->defval; - return 0; + value |= opt->defval; + return set_int_value(opt, flags, value); + } case OPTION_NEGBIT: + { + intmax_t value = get_int_value(opt, flags); if (unset) - *(int *)opt->value |= opt->defval; + value |= opt->defval; else - *(int *)opt->value &= ~opt->defval; - return 0; + value &= ~opt->defval; + return set_int_value(opt, flags, value); + } case OPTION_BITOP: + { + intmax_t value = get_int_value(opt, flags); if (unset) BUG("BITOP can't have unset form"); - *(int *)opt->value &= ~opt->extra; - *(int *)opt->value |= opt->defval; - return 0; + value &= ~opt->extra; + value |= opt->defval; + return set_int_value(opt, flags, value); + } case OPTION_COUNTUP: - if (*(int *)opt->value < 0) - *(int *)opt->value = 0; - *(int *)opt->value = unset ? 0 : *(int *)opt->value + 1; - return 0; + { + size_t bits = CHAR_BIT * opt->precision; + intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits); + intmax_t value = get_int_value(opt, flags); + + if (value < 0) + value = 0; + if (unset) + value = 0; + else if (value < upper_bound) + value++; + else + return error(_("value for %s exceeds %"PRIdMAX), + optname(opt, flags), upper_bound); + return set_int_value(opt, flags, value); + } case OPTION_SET_INT: - *(int *)opt->value = unset ? 0 : opt->defval; - return 0; + return set_int_value(opt, flags, unset ? 0 : opt->defval); case OPTION_STRING: if (unset) @@ -199,23 +277,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"), arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound); - switch (opt->precision) { - case 1: - *(int8_t *)opt->value = value; - return 0; - case 2: - *(int16_t *)opt->value = value; - return 0; - case 4: - *(int32_t *)opt->value = value; - return 0; - case 8: - *(int64_t *)opt->value = value; - return 0; - default: - BUG("invalid precision for option %s", - optname(opt, flags)); - } + return set_int_value(opt, flags, value); } case OPTION_UNSIGNED: { @@ -266,7 +328,9 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, } struct parse_opt_cmdmode_list { - int value, *value_ptr; + intmax_t value; + void *value_ptr; + size_t precision; const struct option *opt; const char *arg; enum opt_parsed flags; @@ -280,7 +344,7 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx, for (; opts->type != OPTION_END; opts++) { struct parse_opt_cmdmode_list *elem = ctx->cmdmode_list; - int *value_ptr = opts->value; + void *value_ptr = opts->value; if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr) continue; @@ -292,10 +356,13 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx, CALLOC_ARRAY(elem, 1); elem->value_ptr = value_ptr; - elem->value = *value_ptr; + elem->precision = opts->precision; + if (do_get_int_value(value_ptr, opts->precision, &elem->value)) + optbug(opts, "has invalid precision"); elem->next = ctx->cmdmode_list; ctx->cmdmode_list = elem; } + BUG_if_bug("invalid 'struct option'"); } static char *optnamearg(const struct option *opt, const char *arg, @@ -317,7 +384,13 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, char *opt_name, *other_opt_name; for (; elem; elem = elem->next) { - if (*elem->value_ptr == elem->value) + intmax_t new_value; + + if (do_get_int_value(elem->value_ptr, elem->precision, + &new_value)) + BUG("impossible: invalid precision"); + + if (new_value == elem->value) continue; if (elem->opt && @@ -327,7 +400,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, elem->opt = opt; elem->arg = arg; elem->flags = flags; - elem->value = *elem->value_ptr; + elem->value = new_value; } if (result || !elem) @@ -586,10 +659,14 @@ static void parse_options_check(const struct option *opts) opts->long_name && !(opts->flags & PARSE_OPT_NONEG)) optbug(opts, "OPTION_SET_INT 0 should not be negatable"); switch (opts->type) { - case OPTION_COUNTUP: + case OPTION_SET_INT: case OPTION_BIT: case OPTION_NEGBIT: - case OPTION_SET_INT: + case OPTION_BITOP: + case OPTION_COUNTUP: + if (!signed_int_fits(opts->defval, opts->precision)) + optbug(opts, "has invalid defval"); + /* fallthru */ case OPTION_NUMBER: if ((opts->flags & PARSE_OPT_OPTARG) || !(opts->flags & PARSE_OPT_NOARG)) diff --git a/parse-options.h b/parse-options.h index 91c3e3c29b..312045604d 100644 --- a/parse-options.h +++ b/parse-options.h @@ -172,6 +172,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG|(f), \ .callback = NULL, \ @@ -182,6 +183,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG|(f), \ } @@ -190,6 +192,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG | (f), \ .defval = (i), \ @@ -238,6 +241,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG|PARSE_OPT_NONEG, \ .defval = (set), \ @@ -248,6 +252,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG, \ .defval = (b), \ @@ -260,6 +265,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, \ .defval = 1, \ @@ -269,6 +275,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .help = (h), \ .flags = PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG | (f), \ .defval = (i), \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index f2663dd0c0..68579d83f3 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -131,6 +131,7 @@ int cmd__parse_options(int argc, const char **argv) .short_name = 'B', .long_name = "no-fear", .value = &boolean, + .precision = sizeof(boolean), .help = "be brave", .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, .defval = 1, @@ -148,9 +149,16 @@ int cmd__parse_options(int argc, const char **argv) OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23), OPT_CMDMODE(0, "mode1", &integer, "set integer to 1 (cmdmode option)", 1), OPT_CMDMODE(0, "mode2", &integer, "set integer to 2 (cmdmode option)", 2), - OPT_CALLBACK_F(0, "mode34", &integer, "(3|4)", - "set integer to 3 or 4 (cmdmode option)", - PARSE_OPT_CMDMODE, mode34_callback), + { + .type = OPTION_CALLBACK, + .long_name = "mode34", + .value = &integer, + .precision = sizeof(integer), + .argh = "(3|4)", + .help = "set integer to 3 or 4 (cmdmode option)", + .flags = PARSE_OPT_CMDMODE, + .callback = mode34_callback, + }, OPT_CALLBACK('L', "length", &integer, "str", "get length of <str>", length_callback), OPT_FILENAME('F', "file", &file, "set file to <file>"), @@ -170,6 +178,7 @@ int cmd__parse_options(int argc, const char **argv) .type = OPTION_COUNTUP, .short_name = '+', .value = &boolean, + .precision = sizeof(boolean), .help = "same as -b", .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH, }, @@ -177,6 +186,7 @@ int cmd__parse_options(int argc, const char **argv) .type = OPTION_COUNTUP, .long_name = "ambiguous", .value = &ambiguous, + .precision = sizeof(ambiguous), .help = "positive ambiguity", .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, }, @@ -184,6 +194,7 @@ int cmd__parse_options(int argc, const char **argv) .type = OPTION_COUNTUP, .long_name = "no-ambiguous", .value = &ambiguous, + .precision = sizeof(ambiguous), .help = "negative ambiguity", .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, }, |