Re: [PATCH v2 01/10] argp-parse.c (__argp_input): Don't crash if pstate is NULL

[Robbie Harwood]

Bruno Haible <bruno@clisp.org> writes:

> Robbie Harwood wrote:
>> From: Colin Watson <cjwatson@ubuntu.com>
>> 
>> [rharwood@redhat.com: tweaked commit message]
>> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
>> ---
>>  lib/argp-parse.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/lib/argp-parse.c b/lib/argp-parse.c
>> index 053495ec0..4f1c65d73 100644
>> --- a/lib/argp-parse.c
>> +++ b/lib/argp-parse.c
>> @@ -940,7 +940,7 @@ weak_alias (__argp_parse, argp_parse)
>>  void *
>>  __argp_input (const struct argp *argp, const struct argp_state *state)
>>  {
>> -  if (state)
>> +  if (state && state->pstate)
>>      {
>>        struct group *group;
>>        struct parser *parser = state->pstate;
>
> I don't think this patch is needed, because:
>
> 1) The application cannot construct a 'struct argp_state' by itself, since [1]
>    says that the 'struct argp_state' contains a member 'pstate' that is
>    "Private, for use by the argp implementation.".
>
> 2) The only place in the gnulib / glibc code where a 'struct argp_state' is
>    being constructed, is in function parser_init, invoked from 'argp_parse',
>    and there a non-NULL value is assigned.
>
> In other words, there is no way, compliant with the documented API, that a
> NULL pointer can arise as state->pstate.
>
> Bruno
>
> [1] https://www.gnu.org/software/libc/manual/html_node/Argp-Parsing-State.html

Thanks for taking a look.  I don't have more information on this one
beyond the little that's in the commit, so unless Colin remembers why
this was needed in 2011, I'll propose grub drop it at some point.

Be well,
--Robbie

signature.asc
Description: PGP signature