Re: [PATCH 2/3] send-pack: fix memory leak around duplicate refs

From: Junio C Hamano <gitster@pobox.com>
Date: May 14, 2025, 17:46 UTC
Message-ID: <xmqqikm39jbp.fsf@gitster.g>
In-Reply-To: <20250514-501-update-git-fetch-1-to-use-partial-transactions-v1-2-7c65f46493d4@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 7 quoted lines

> The 'git-send-pack(1)' allows users to push objects to a remote
> repository and explicitly list the references to be pushed. The status
> of each reference pushed is captured into a list mapped by refname.
>
> If a reference fails to be updated, its error message is captured in the
> `ref->remote_status` field. While the command allows duplicate ref
> inputs, the list of doesn't accommodate this behavior as a particular

-ECANNOTPARSE around "the list of doesn't accommodate".

Show 10 quoted lines

> refname is linked to a single `struct ref*` element. So if the user
> inputs a reference twice like:
>
>   git send-pack remote.git A:foo B:foo
>
> where the user is trying to update the same reference 'foo' twice and
> the reference fails to be updated, we first fill `ref->remote_status`
> with error message for the input 'A:foo' then we override the same field
> with the error message for 'B:foo'. This override happens without first
> free'ing the previous value. Fix this leak.

OK. A natural question is what happens when A successfully updates and B fails, or A fails but B successfully updates (failing both is much less interesting). What should happen in such cases is unclear but my gut feeling is that the last-one wins (which you implemented) is probably just as OK as the first-one gets retained (which might be less work at runtime), and perhaps keeping-the-more-severe-one might be more useful than either of these two, but I didn't think it through.

THanks.