Re: [PATCH 2/5] parse-options: introduce precision handling for `OPTION_INTEGER`
- From
René Scharfe <l.s.r@web.de>
- Date
- Apr 1, 2025, 18:47 UTC
- Message-ID
- <f6c7c92b-931c-4be4-8d76-9d4ed147e983@web.de>
- In-Reply-To
- <20250401-b4-pks-parse-options-integers-v1-2-a628ad40c3b4@pks.im>
Am 01.04.25 um 17:01 schrieb Patrick Steinhardt:
Show 10 quoted lines
> diff --git a/parse-options.c b/parse-options.c
> index 35fbb3b0d63..dbda9b7cfe7 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -172,25 +172,50 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
> return (*opt->ll_callback)(p, opt, p_arg, p_unset);
> }
> case OPTION_INTEGER:
> + {
> + intmax_t upper_bound = (((intmax_t) 1 << (opt->precision * 8 - 1)) - 1);Ugh, how does this not overflow? The macro maximum_signed_value_of_type does a similar calculation better.
> + intmax_t lower_bound = -upper_bound - 1;
This depends on two's complement being used, which is bad for purity and portability to obsolete machines, but probably OK in practice.
Show 50 quoted lines
> + intmax_t value;
> +
> if (unset) {
> - *(int *)opt->value = 0;
> - return 0;
> - }
> - if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
> - *(int *)opt->value = opt->defval;
> - return 0;
> - }
> - if (get_arg(p, opt, flags, &arg))
> + value = 0;
> + } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
> + value = opt->defval;
> + } else if (get_arg(p, opt, flags, &arg)) {
> return -1;
> - if (!*arg)
> + } else if (!*arg) {
> return error(_("%s expects a numerical value"),
> optname(opt, flags));
> - *(int *)opt->value = strtol(arg, (char **)&s, 10);
> - if (*s)
> - return error(_("%s expects a numerical value"),
> - optname(opt, flags));
> - return 0;
> + } else {
> + value = strtoimax(arg, (char **)&s, 10);
> + if (*s)
> + return error(_("%s expects a numerical value"),
> + optname(opt, flags));
> +
> + }
>
> + if (value < lower_bound || value > upper_bound)
> + return error(_("value %"PRIdMAX" for %s not in range [%"PRIdMAX",%"PRIdMAX"]"),
> + value, optname(opt, flags), lower_bound, 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;Do we even need all these sizes? Or can we whittle it down to ssize_t?
And for which quantities do we need to accept negative values anyway?
René