On 02/26, Jonathan Nieder wrote:
> Brandon Williams wrote:
>
> > Remove code duplication and use the existing 'get_refs_via_connect()'
> > function to retrieve a remote's heads in 'fetch_refs_via_pack()' and
> > 'git_transport_push()'.
> >
> > Signed-off-by: Brandon Williams <bmw...@google.com>
> > ---
> > transport.c | 18 ++++--------------
> > 1 file changed, 4 insertions(+), 14 deletions(-)
>
> I like the diffstat.
>
> [...]
> > +++ 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?
>
> > @@ -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.
>
> Same question as the other caller about whether we mind getting
> extra_have populated.
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.
--
Brandon Williams
