git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: "D. Ben Knoble" <ben.knoble+github@gmail.com>,
	 git@vger.kernel.org, Phillip Wood <phillip.wood@dunelm.org.uk>,
	 Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH 1/5] parseopt: fix :(optional) at command line to only ignore missing files
Date: Tue, 04 Nov 2025 09:34:20 -0800	[thread overview]
Message-ID: <xmqqwm45puqr.fsf@gitster.g> (raw)
In-Reply-To: <xmqq1pmdr9qu.fsf@gitster.g> (Junio C. Hamano's message of "Tue, 04 Nov 2025 09:24:57 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> Hi Ben
>>
>> These all look good to me though I agree with Junio's comments on patch 
>> 3. It would be nice to get at least the fist patch merged in time for 
>> 2.52.0.
>
> Yup, let me do exactly that ;-)
>
> Thanks, both.

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.

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);
-- 
2.52.0-rc0-28-g4cf919bd7b


  reply	other threads:[~2025-11-04 17:34 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-02 16:17 [PATCH 0/5] Fixes for :(optional) path code D. Ben Knoble
2025-11-02 16:17 ` [PATCH 1/5] parseopt: fix :(optional) at command line to only ignore missing files D. Ben Knoble
2025-11-04 16:19   ` Phillip Wood
2025-11-04 17:24     ` Junio C Hamano
2025-11-04 17:34       ` Junio C Hamano [this message]
2025-11-04 18:24         ` D. Ben Knoble
2025-11-05 16:35         ` Phillip Wood
2025-11-06 17:47           ` Junio C Hamano
2025-11-02 16:17 ` [PATCH 2/5] doc: clarify command equivalence comment D. Ben Knoble
2025-11-02 16:17 ` [PATCH 3/5] parseopt: use boolean type for a simple flag D. Ben Knoble
2025-11-03  5:19   ` Junio C Hamano
2025-11-04 16:21     ` Phillip Wood
2025-11-04 18:22       ` D. Ben Knoble
2025-11-02 16:17 ` [PATCH 4/5] config: " D. Ben Knoble
2025-11-02 16:17 ` [PATCH 5/5] parseopt: restore const qualifier to parsed filename D. Ben Knoble

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqwm45puqr.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=ben.knoble+github@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=phillip.wood123@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).