Brandon Williams wrote:

On 02/26, Jonathan Nieder wrote:

quoted

Brandon Williams wrote:

quoted

quoted

+++ b/transport.c

@@ -230,12 +230,8 @@ static int fetch_refs_via_pack(struct transport *transport,
 	args.cloning = transport->cloning;
 	args.update_shallow = data->options.update_shallow;
 
-	if (!data->got_remote_heads) {
-		connect_setup(transport, 0);
-		get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0,
-				 NULL, &data->shallow);
-		data->got_remote_heads = 1;
-	}
+	if (!data->got_remote_heads)
+		refs_tmp = get_refs_via_connect(transport, 0);

The only difference between the old and new code is that the old code
passes NULL as 'extra_have' and the new code passes &data->extra_have.

That means this populates the data->extra_have oid_array.  Does it
matter?

[...]

I don't think its a problem to have extra_have populated, least I
haven't seen anything to lead me to believe it would be a problem.

Assuming it gets properly freed later, the only effect I can imagine
is some increased memory usage.

I'm inclined to agree with you that the simplicity is worth it.  It
seems worth mentioning in the commit message, though.

[...]

quoted

quoted

@@ -541,14 +537,8 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 	struct send_pack_args args;
 	int ret;
 
-	if (!data->got_remote_heads) {
-		struct ref *tmp_refs;
-		connect_setup(transport, 1);
-
-		get_remote_heads(data->fd[0], NULL, 0, &tmp_refs, REF_NORMAL,
-				 NULL, &data->shallow);
-		data->got_remote_heads = 1;
-	}
+	if (!data->got_remote_heads)
+		get_refs_via_connect(transport, 1);

not a new problem, just curious: Does this leak tmp_refs?

Maybe, though its removed by this patch.

Sorry for the lack of clarity.  If it was leaked before, then it is
still leaked now, via the discarded return value from
get_refs_via_connect.

Any idea how we can track that down?  E.g. are there ways to tell leak
checkers "just tell me about this particular allocation"?

Thanks,
Jonathan