From: Junio C Hamano Date: Wed, 14 May 2025 17:46:18 GMT Subject: Re: [PATCH 2/3] send-pack: fix memory leak around duplicate refs Message-ID: In-Reply-To: <20250514-501-update-git-fetch-1-to-use-partial-transactions-v1-2-7c65f46493d4@gmail.com> Karthik Nayak writes: > 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". > 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.