Brandon Williams wrote:
> On 02/26, Jonathan Nieder wrote:
>> Brandon Williams wrote:
>>> +++ 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.
[...]
>>> @@ -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
