Re: [PATCH v3 06/35] transport: use get_refs_via_connect to get refs
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Feb 27, 2018, 06:08 UTC
- Message-ID
- <20180227060840.GD65699@aiede.svl.corp.google.com>
- In-Reply-To
- <20180207011312.189834-7-bmwill@google.com>
Brandon Williams wrote:
Show 8 quoted lines
> 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 <bmwill@google.com> > --- > transport.c | 18 ++++-------------- > 1 file changed, 4 insertions(+), 14 deletions(-)
I like the diffstat.
[...]
Show 13 quoted lines
> +++ 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?
Show 14 quoted lines
> @@ -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