From: Christian Couder <christian.couder@gmail.com>
Date: Mon, 02 Nov 2026 15:25:30 GMT
Subject: Re: [PATCH v3 3/4] parse-options: add early_scan_options()
Message-ID: <CAP8UFD1P1=HhuiKEDjBKdjauKUbeBYG6gT2_WZfq0qh93MN-LQ@mail.gmail.com>
In-Reply-To: <6782fa63-1e46-4ae9-b129-a157986c2556@gmail.com>

On Fri, Oct 30, 2026 at 10:27 PM Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
>
> On 10/12/26 13:14, Christian Couder wrote:

> > @@ -763,6 +765,8 @@ static void parse_options_check(const struct option *opts)
> >               case OPTION_SUBCOMMAND:
> >                       if (!opts->value || !opts->subcommand_fn)
> >                               optbug(opts, "OPTION_SUBCOMMAND needs a value and a subcommand function");
> > +                     if (opts->flags & PARSE_OPT_EARLY)
> > +                             optbug(opts, "OPTION_SUBCOMMAND does not support PARSE_OPT_EARLY");
> >                       if (!subcommand_value)
> >                               subcommand_value = opts->value;
> >                       else if (subcommand_value != opts->value)
>
> Would it make sense to also have these validations within
> early_scan_options() just in case it is called before
> parse_options_start() is invoked?

Yeah, I think it's cheap and simple enough to just call
parse_options_check() from early_scan_options(), so I have added that
call in the v4 I just sent.


> > +/*
> > + * Called by early_scan_options() for each argument matching a
> > + * `struct option` with PARSE_OPT_EARLY set.
> > + *
> > + * - `option` is the matching option,
>
> Would it be helpful to also convey here that the caller is not expected
> to retain the pointer as the memory pointing to it would be freed once
> the scan completes?

Yeah, it looks like this in v4:

 * - `option` is the matching option, (may point into a temporary
 *   copy of the options, so must not be used after the scan,)

Thanks.