Re: [PATCH] send-pack: clean up extra_have oid array

On Mon, Jun 30, 2025 at 7:31 AM Junio C Hamano [off-list ref] wrote:

Jacob Keller [off-list ref] writes:

quoted

From: Jacob Keller <redacted>

Commit c8009635785e ("fetch-pack, send-pack: clean up shallow oid
array", 2024-09-25) cleaned up the shallow oid array in cmd_send_pack,
but didn't clean up extra_have, which is still leaked at program exit.
I suspect the particular tests in t5539 don't trigger any additions to
the extra_have array, which explains why the tests can pass leak free
despite this gap.

Signed-off-by: Jacob Keller <redacted>
---
I didn't check to see why the t5539 tests don't leak. This leak occured for
me in a day-to-day run with my local git build that happened to still have
sanitizers enabled:

The other side may tell you about objects you _cannot_ fetch from
them, but if you have them, these objects can participate in the
common ancestor discovery and reduce the size of the transfer.

If the repository A you are pushing into use an alternate object
store B (i.e., created by "git clone --reference B $URL A" to make A
borrow from another local repository B) for example, the refs in
that alternate B that point at objects not in the repository A are
shown as "extra" objects.

Perhaps we can have these tests push into such a repository?

I probably won't personally have time to work on extending these tests.

Thanks,
Jake

quoted

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index c6e0e9d05186..61486e378cab 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c

@@ -343,6 +343,7 @@ int cmd_send_pack(int argc,
      free_refs(remote_refs);
      free_refs(local_refs);
      refspec_clear(&rs);
+     oid_array_clear(&extra_have);
      oid_array_clear(&shallow);
      clear_cas_option(&cas);
      return ret;

The change looks obviously correct.

Thanks.