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 <bmwill@xxxxxxxxxx> > --- > 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