Re: [PATCH 1/5] parseopt: fix :(optional) at command line to only ignore missing files

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 04/11/2025 17:34, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> Let me have this on top of Ben's 5-patch series.
>> 
>> ----- >8 -----
>> Subject: [PATCH] parseopt: remove unreachable code
>> 
>> At this point in the code after running skip_prefix() on the
>> variable and receiving the result in the same variable, the contents
>> of the variable can never be NULL.  The function either (1) updates
>> the variable to point at a later part of the string it originally
>> pointed at, or (2) leaves it intact if the string does not have the
>> prefix.  (1) will never make the variable NULL, and (2) cannot be
>> the source of NULL, because the variable cannot be NULL before
>> calling skip_prefix(), which would die immediately by dereferencing
>> the NULL pointer in that case.
>
> Nicely explained, the changes below look good
>
> Thanks
>
> Phillip

Thanks for a quick review.

>
>> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>   parse-options.c | 2 --
>>   1 file changed, 2 deletions(-)
>> 
>> diff --git a/parse-options.c b/parse-options.c
>> index 27c1e75d53..97a55300e8 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -223,8 +223,6 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
>>   			return 0;
>>   
>>   		is_optional = skip_prefix(value, ":(optional)", &value);
>> -		if (!value)
>> -			is_optional = false;
>>   		value = fix_filename(p->prefix, value);
>>   		if (is_optional && is_missing_file(value)) {
>>   			free((char *)value);