On Tue, Jul 01, 2025 at 05:21:11PM +0200, René Scharfe wrote:
> On 7/1/25 12:55 PM, Patrick Steinhardt wrote:
> > On Sun, Jun 29, 2025 at 01:51:19PM +0200, René Scharfe wrote:
> >> Similar to 09705696f7 (parse-options: introduce precision handling for
> >> `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
> >> for OPTION_BITOP. Do that by requiring their "precision" to be set,
> >> casting their "value" pointer accordingly and checking whether the value
> >> fits.
> >>
> >> Checking "defval" has the side-effect of also requiring PARSE_OPT_NOARG.
> >
> > Hm, requiring PARSE_OPT_NOARG for what? I cannot see it being touched in
> > this patch at all, so I'm a but puzzled.
>
> For options with OPTION_BITOP. Adding the defval check also adds the
> no-argument check by falling through to it:
>
> diff --git a/parse-options.c b/parse-options.c
> index 6bd7158806..0dc9b0324a 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -620,18 +623,19 @@ static void parse_options_check(const struct option *opts)
> optbug(opts, "uses feature "
> "not supported for dashless options");
> if (opts->type == OPTION_SET_INT && !opts->defval &&
> opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
> optbug(opts, "OPTION_SET_INT 0 should not be negatable");
> switch (opts->type) {
> case OPTION_SET_INT:
> case OPTION_BIT:
> case OPTION_NEGBIT:
> + case OPTION_BITOP:
> if (!signed_int_fits(opts->defval, opts->precision))
> optbug(opts, "has invalid defval");
> /* fallthru */
> case OPTION_COUNTUP:
> case OPTION_NUMBER:
> if ((opts->flags & PARSE_OPT_OPTARG) ||
> !(opts->flags & PARSE_OPT_NOARG))
> optbug(opts, "should not accept an argument");
> break;
Ah, now I see it, the extended context definitely helps. Might be nice
to point out explicitly in the commit message that it's about the
fallthrough behaviour.
Patrick
