From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 05 Oct 2026 13:07:27 GMT
Subject: Re: [PATCH v5] fetch.c: defer fetch.followRemoteHEAD validation
Message-ID: <xmqqqzi4nvn4.fsf@gitster.g>
In-Reply-To: <20261004201428.5210-1-colinlewishinton@gmail.com>

Colin Hinton <colinlewishinton@gmail.com> writes:

I see there are only two minor things remaining in this iteration.

> fetch. Leave NEEDSWORK comments at both the now unresolved call site
> in do_fetch() and at the actual defect in handle_config(), so the
> remaining scope is easy to find for a follow-up patch.

Here is one of the two.  There is only one NEEDSWORK, not "at both".

	Leave a NEEDSWORK comment at remote.c:handle_config() that
	has a defect similar to what is fixed by this patch, so ...

should be sufficient.

Another is that 

        int cmd_fetch(int argc,
                      const char **argv,
                      const char *prefix,
                      struct repository *repo UNUSED)
        {
                struct fetch_config config = {
                        .display_format = DISPLAY_FORMAT_FULL,
                        .follow_remote_head_raw = NULL,
                        .follow_remote_head_seen = 0,
                        .prune = -1,
                        .prune_tags = -1,
                        .show_forced_updates = 1,
                        .recurse_submodules = RECURSE_SUBMODULES_DEFAULT,
                        .parallel = 1,
                        .submodule_fetch_jobs = -1,
                };

will hold onto a copy of config.follow_remote_head_seen that was
read from the configuration and never frees it, so when cmd_fetch()
leaves, it technically leaks a string.

Other than these two points, this round looks very good.

Thanks.