Re: Bug in fetch-pack.c, please confirm

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>>  - The only caller of everything_local(), do_fetch_pack(), returns
>>    this list of ref, whose element has bogus new_sha1 values, to its
>>    caller.  It does not look at the elements and acts on them.
>
> I'm not sure what the final sentence means. Should it be "It does not
> look at the elements nor act on them"?

Yes.  "It does not look at the new_sha1.  It does not use the
information to change its behaviour.  Both of the previous two
statements are true." is what I meant.

> do_fetch_pack actually does pass them down to find_common. But the
> latter does not look at ref->new_sha1, so we're OK.

>>  - The other caller of fetch_pack() is fetch_refs_via_pack() in the
>>    transport layer, which is a helper that implements "git fetch".
>>    It only cares about whether the returned list is empty (i.e.
>>    failed to fetch anything).
>
> So I thought I would follow this up by adding a free_refs() call in
> fetch_refs_via_pack. After all, we just leak that list, right?

Yeah, I agree.

> I'm working up a few patches in that area, which I'll send out in a few
> minutes. Once that is done, then I think the explanation you give above
> would be correct.

If a follow-up is coming then I'd just drop this one.  Thanks.