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?

Same question as the other caller about whether we mind getting
extra_have populated.

Thanks,
Jonathan

Reply via email to