From: Junio C Hamano Date: Thu, 06 Nov 2025 17:47:36 GMT Subject: Re: [PATCH 1/5] parseopt: fix :(optional) at command line to only ignore missing files Message-ID: In-Reply-To: <5951a930-0e57-4201-9b56-12a41cb44333@gmail.com> Phillip Wood writes: > 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 Thanks for a quick review. > >> 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);