Re: [PATCH v3 3/7] parse-options: introduce precision handling for `OPTION_INTEGER`

> The `OPTION_INTEGER` option type accepts a signed integer. The type of
> the underlying integer is a simple `int`, which restricts the range of
> values accepted by such options. But there is a catch: because the
> caller provides a pointer to the value via the `.value` field, which is
> a simple void pointer. This has two consequences:
>
>   - There is no check whether the passed value is sufficiently long to
>     store the entire range of `int`. This can lead to integer wraparound
>     in the best case and out-of-bounds writes in the worst case.
>
>   - Even when a caller knows that they want to store a value larger than
>     `INT_MAX` they don't have a way to do so.
>
> Funny enough, even if the caller gets everything correct the parsing
> logic is still insufficient because we use `strtol()` to parse the
> argument, which returns a `long`. But as that value is implicitly cast
> when assigning it to the `int` field we may still get invalid results.
>
> In practice this doesn't tend to be a huge issue because users typically
> don't end up passing huge values to most commands. But the parsing logic
> is demonstrably broken, and it is too easy to get the calling convention
> wrong.
>
> Improve the situation by introducing a new `precision` field into the
> structure. This field gets assigned automatically by `OPT_INTEGER_F()`
> and tracks the size of the passed value. Like this it becomes possible
> for the caller to pass arbitrarily-sized integers and the underlying
> logic knows to handle it correctly by doing range checks. Furthermore,
> convert the code to use `strtoimax()` intstead of `strtol()` so that we
> can also parse values larger than `LONG_MAX`.
>
> Note that we do not yet assert signedness of the passed variable, which
> is another source of bugs. This will be handled in a subsequent commit.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> ...
> +	{
> +		intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - CHAR_BIT * opt->precision);
> +		intmax_t lower_bound = -upper_bound - 1;
> +		intmax_t value;
> +
>  		if (unset) {
> ...
> +		if (value < lower_bound || value > upper_bound)
>  			return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"),

> +		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));
> +		}
> +	}
>  	case OPTION_MAGNITUDE:
>  		if (unset) {
>  			*(unsigned long *)opt->value = 0;
> diff --git a/parse-options.h b/parse-options.h
> index 997ffbee805..8db96402c4d 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -92,6 +92,10 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv,
>   * `value`::
>   *   stores pointers to the values to be filled.
>   *
> + * `precision`::
> + *   precision of the integer pointed to by `value` in number of bytes. Should
> + *   typically be its `sizeof()`.
> + *

> @@ -214,6 +219,7 @@ struct option {
>  	.short_name = (s), \
>  	.long_name = (l), \
>  	.value = (v), \
> +	.precision = sizeof(*v), \
>  	.argh = N_("n"), \
>  	.help = (h), \
>  	.flags = (f), \