Re: [PATCH 5/6] parse-options: add precision handling for OPTION_BITOP

[Patrick Steinhardt <ps@pks.im>]

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