Extend precision handling to all options that currently blindly use an
int pointer to access value variables. This allows the safe use of
smaller and larger integer types.
Sign handling might be nice as well (especially for OPTION_COUNTUP), but
is out of scope for this series.
Changes since v1:
- Enable PARSE_OPT_NOARG checking for OPTION_BITOP explicitly in a new
separate patch instead of as a side-effect.
- Split out do_get_int_value().
- This allows us to use optbug() only where it makes sense, in
build_cmdmode_list(). This lets the function report all
PARSE_OPT_CMDMODE options with invalid precision in one go.
- Use the same BUG call in get_int_value() as in set_int_value(),
for consistency.
- Store the precision in build_cmdmode_list() instead of an example
option, which is hopefully easier to follow.
- Rename the size parameter of signed_int_fits() for consistency.
parse-options: require PARSE_OPT_NOARG for OPTION_BITOP
parse-options: add precision handling for PARSE_OPT_CMDMODE
parse-options: add precision handling for OPTION_SET_INT
parse-options: add precision handling for OPTION_BIT
parse-options: add precision handling for OPTION_NEGBIT
parse-options: add precision handling for OPTION_BITOP
parse-options: add precision handling for OPTION_COUNTUP
builtin/am.c | 1 +
builtin/rebase.c | 1 +
builtin/update-index.c | 6 ++
builtin/write-tree.c | 1 +
parse-options.c | 155 +++++++++++++++++++++++++---------
parse-options.h | 7 ++
t/helper/test-parse-options.c | 17 +++-
7 files changed, 146 insertions(+), 42 deletions(-)
Interdiff against v1:
diff --git a/parse-options.c b/parse-options.c
index 0dd08a3a77..5224203ffe 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -68,23 +68,34 @@ static char *fix_filename(const char *prefix, const char *file)
return prefix_filename_except_for_dash(prefix, file);
}
-static intmax_t get_int_value(const struct option *opt)
+static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
{
- switch (opt->precision) {
+ switch (precision) {
case sizeof(int8_t):
- return *(int8_t *)opt->value;
+ *ret = *(int8_t *)value;
+ return 0;
case sizeof(int16_t):
- return *(int16_t *)opt->value;
+ *ret = *(int16_t *)value;
+ return 0;
case sizeof(int32_t):
- return *(int32_t *)opt->value;
+ *ret = *(int32_t *)value;
+ return 0;
case sizeof(int64_t):
- return *(int64_t *)opt->value;
+ *ret = *(int64_t *)value;
+ return 0;
default:
- optbug(opt, "has invalid precision");
- BUG("invalid 'struct option'");
+ 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)
@@ -107,9 +118,9 @@ static enum parse_opt_result set_int_value(const struct option *opt,
}
}
-static int signed_int_fits(intmax_t value, size_t size)
+static int signed_int_fits(intmax_t value, size_t precision)
{
- size_t bits = size * CHAR_BIT;
+ 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;
@@ -137,7 +148,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
case OPTION_BIT:
{
- intmax_t value = get_int_value(opt);
+ intmax_t value = get_int_value(opt, flags);
if (unset)
value &= ~opt->defval;
else
@@ -147,7 +158,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
case OPTION_NEGBIT:
{
- intmax_t value = get_int_value(opt);
+ intmax_t value = get_int_value(opt, flags);
if (unset)
value |= opt->defval;
else
@@ -157,7 +168,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
case OPTION_BITOP:
{
- intmax_t value = get_int_value(opt);
+ intmax_t value = get_int_value(opt, flags);
if (unset)
BUG("BITOP can't have unset form");
value &= ~opt->extra;
@@ -169,7 +180,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
{
size_t bits = CHAR_BIT * opt->precision;
intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
- intmax_t value = get_int_value(opt);
+ intmax_t value = get_int_value(opt, flags);
if (value < 0)
value = 0;
@@ -318,7 +329,9 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
struct parse_opt_cmdmode_list {
intmax_t value;
- const struct option *opt, *reference_opt;
+ void *value_ptr;
+ size_t precision;
+ const struct option *opt;
const char *arg;
enum opt_parsed flags;
struct parse_opt_cmdmode_list *next;
@@ -331,21 +344,25 @@ 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;
+ void *value_ptr = opts->value;
- if (!(opts->flags & PARSE_OPT_CMDMODE) || !opts->value)
+ if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr)
continue;
- while (elem && elem->reference_opt->value != opts->value)
+ while (elem && elem->value_ptr != value_ptr)
elem = elem->next;
if (elem)
continue;
CALLOC_ARRAY(elem, 1);
- elem->reference_opt = opts;
- elem->value = get_int_value(opts);
+ elem->value_ptr = 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,
@@ -367,7 +384,11 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
char *opt_name, *other_opt_name;
for (; elem; elem = elem->next) {
- intmax_t new_value = get_int_value(elem->reference_opt);
+ 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;
--
2.50.0
