From: Phillip Wood Date: Wed, 05 Nov 2025 16:35:25 GMT Subject: Re: [PATCH 1/5] parseopt: fix :(optional) at command line to only ignore missing files Message-ID: <5951a930-0e57-4201-9b56-12a41cb44333@gmail.com> In-Reply-To: On 04/11/2025 17:34, Junio C Hamano wrote: > Junio C Hamano 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 > Helped-by: Phillip Wood > Signed-off-by: Junio C Hamano > --- > 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);