remote: make _ls return the list directly

The callback-based method of listing remote references dates back to the
beginning of the network code's lifetime, when we didn't know any
better.

We need to keep the list around for update_tips() after disconnect() so
let's make use of this to simply give the user a pointer to the array so
they can write straightforward code instead of having to go through a
callback.

8 files changed, 71 insertions, 84 deletions

diff --git a/src/clone.c b/src/clone.c
index f9338b746..23aacd478 100644
--- a/src/clone.c
+++ b/src/clone.c
@@ -176,25 +176,20 @@ static int update_head_to_new_branch(
 	return error;
 }
 
-static int get_head_callback(git_remote_head *head, void *payload)
-{
-	git_remote_head **destination = (git_remote_head **)payload;
-
-	/* Save the first entry, and terminate the enumeration */
-	*destination = head;
-	return 1;
-}
-
 static int update_head_to_remote(git_repository *repo, git_remote *remote)
 {
 	int retcode = -1;
+	size_t refs_len;
 	git_refspec dummy_spec;
-	git_remote_head *remote_head;
+	const git_remote_head *remote_head, **refs;
 	struct head_info head_info;
 	git_buf remote_master_name = GIT_BUF_INIT;
 
+	if (git_remote_ls(&refs, &refs_len, remote) < 0)
+		return -1;
+
 	/* Did we just clone an empty repository? */
-	if (remote->refs.length == 0) {
+	if (refs_len == 0) {
 		return setup_tracking_config(
 			repo,
 			"master",
@@ -202,12 +197,8 @@ static int update_head_to_remote(git_repository *repo, git_remote *remote)
 			GIT_REFS_HEADS_MASTER_FILE);
 	}
 
-	/* Get the remote's HEAD. This is always the first ref in remote->refs. */
-	remote_head = NULL;
-
-	if (!remote->transport->ls(remote->transport, get_head_callback, &remote_head))
-		return -1;
-
+	/* Get the remote's HEAD. This is always the first ref in the list. */
+	remote_head = refs[0];
 	assert(remote_head);
 
 	git_oid_cpy(&head_info.remote_head_oid, &remote_head->oid);
diff --git a/src/fetch.c b/src/fetch.c
index 295b036ef..19afddcd8 100644
--- a/src/fetch.c
+++ b/src/fetch.c
@@ -19,52 +19,47 @@
 #include "repository.h"
 #include "refs.h"
 
-struct filter_payload {
-	git_remote *remote;
-	const git_refspec *spec, *tagspec;
-	git_odb *odb;
-};
-
-static int filter_ref__cb(git_remote_head *head, void *payload)
+static int maybe_want(git_remote *remote, git_remote_head *head, git_odb *odb, git_refspec *tagspec)
 {
-	struct filter_payload *p = payload;
 	int match = 0;
 
 	if (!git_reference_is_valid_name(head->name))
 		return 0;
 
-	if (p->remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
+	if (remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
 		/*
 		 * If tagopt is --tags, then we only use the default
 		 * tags refspec and ignore the remote's
 		 */
-		if (git_refspec_src_matches(p->tagspec, head->name))
+		if (git_refspec_src_matches(tagspec, head->name))
 			match = 1;
 		else
 			return 0;
-	} else if (git_remote__matching_refspec(p->remote, head->name))
+	} else if (git_remote__matching_refspec(remote, head->name))
 			match = 1;
 
 	if (!match)
 		return 0;
 
 	/* If we have the object, mark it so we don't ask for it */
-	if (git_odb_exists(p->odb, &head->oid))
+	if (git_odb_exists(odb, &head->oid))
 		head->local = 1;
 	else
-		p->remote->need_pack = 1;
+		remote->need_pack = 1;
 
-	return git_vector_insert(&p->remote->refs, head);
+	return git_vector_insert(&remote->refs, head);
 }
 
 static int filter_wants(git_remote *remote)
 {
-	struct filter_payload p;
+	git_remote_head **heads;
 	git_refspec tagspec, head;
-	int error = -1;
+	int error = 0;
+	git_odb *odb;
+	size_t i, heads_len;
 
-	//git_vector_clear(&remote->refs);
-	if (git_refspec__parse(&tagspec, GIT_REFSPEC_TAGS, true) < 0)
+	git_vector_clear(&remote->refs);
+	if ((error = git_refspec__parse(&tagspec, GIT_REFSPEC_TAGS, true)) < 0)
 		return error;
 
 	/*
@@ -73,8 +68,6 @@ static int filter_wants(git_remote *remote)
 	 * not interested in any particular branch but just the remote's
 	 * HEAD, which will be stored in FETCH_HEAD after the fetch.
 	 */
-	p.tagspec = &tagspec;
-	p.remote = remote;
 	if (remote->active_refspecs.length == 0) {
 		if ((error = git_refspec__parse(&head, "HEAD", true)) < 0)
 			goto cleanup;
@@ -83,12 +76,16 @@ static int filter_wants(git_remote *remote)
 			goto cleanup;
 	}
 
-	git_vector_clear(&remote->refs);
+	if (git_repository_odb__weakptr(&odb, remote->repo) < 0)
+		goto cleanup;
 
-	if (git_repository_odb__weakptr(&p.odb, remote->repo) < 0)
+	if (git_remote_ls((const git_remote_head ***)&heads, &heads_len, remote) < 0)
 		goto cleanup;
 
-	error = git_remote_ls(remote, filter_ref__cb, &p);
+	for (i = 0; i < heads_len; i++) {
+		if ((error = maybe_want(remote, heads[i], odb, &tagspec)) < 0)
+			break;
+	}
 
 cleanup:
 	git_refspec__free(&tagspec);
diff --git a/src/push.c b/src/push.c
index a799db8d0..4f442c6a2 100644
--- a/src/push.c
+++ b/src/push.c
@@ -616,16 +616,22 @@ on_error:
 	return error;
 }
 
-static int cb_filter_refs(git_remote_head *ref, void *data)
-{
-	git_remote *remote = (git_remote *) data;
-	return git_vector_insert(&remote->refs, ref);
-}
-
 static int filter_refs(git_remote *remote)
 {
+	const git_remote_head **heads;
+	size_t heads_len, i;
+
 	git_vector_clear(&remote->refs);
-	return git_remote_ls(remote, cb_filter_refs, remote);
+
+	if (git_remote_ls(&heads, &heads_len, remote) < 0)
+		return -1;
+
+	for (i = 0; i < heads_len; i++) {
+		if (git_vector_insert(&remote->refs, heads[i]) < 0)
+			return -1;
+	}
+
+	return 0;
 }
 
 int git_push_finish(git_push *push)
diff --git a/src/remote.c b/src/remote.c
index a2fd21007..289ead802 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -620,11 +620,11 @@ on_error:
 	return -1;
 }
 
-int git_remote_ls(git_remote *remote, git_headlist_cb list_cb, void *payload)
+int git_remote_ls(const git_remote_head ***out, size_t *size, git_remote *remote)
 {
 	assert(remote);
 
-	return remote->transport->ls(remote->transport, list_cb, payload);
+	return remote->transport->ls(out, size, remote->transport);
 }
 
 int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_url)
@@ -684,13 +684,6 @@ int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_ur
 	return 0;
 }
 
-static int store_refs(git_remote_head *head, void *payload)
-{
-	git_vector *refs = (git_vector *)payload;
-
-	return git_vector_insert(refs, head);
-}
-
 /* DWIM `refspecs` based on `refs` and append the output to `out` */
 static int dwim_refspecs(git_vector *out, git_vector *refspecs, git_vector *refs)
 {
@@ -736,15 +729,12 @@ int git_remote_download(git_remote *remote)
 	if (git_vector_init(&refs, 8, remote_head_cmp) < 0)
 		return -1;
 
-	if (git_remote_ls(remote, store_refs, &refs) < 0) {
+	if (git_remote_ls((const git_remote_head ***)&refs.contents, &refs.length, remote) < 0)
 		return -1;
-	}
 
 	free_refspecs(&remote->active_refspecs);
 
-	error = dwim_refspecs(&remote->active_refspecs, &remote->refspecs, &refs);
-	git_vector_free(&refs);
-	if (error < 0)
+	if (dwim_refspecs(&remote->active_refspecs, &remote->refspecs, &refs) < 0)
 		return -1;
 
 	if ((error = git_fetch_negotiate(remote)) < 0)
@@ -996,7 +986,7 @@ int git_remote_update_tips(git_remote *remote)
 	if (git_vector_init(&refs, 16, NULL) < 0)
 		return -1;
 
-	if ((error = git_remote_ls(remote, store_refs, &refs)) < 0)
+	if ((error = git_remote_ls((const git_remote_head ***)&refs.contents, &refs.length, remote)) < 0)
 		goto out;
 
 	if (remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
@@ -1014,7 +1004,6 @@ int git_remote_update_tips(git_remote *remote)
 
 out:
 	git_refspec__free(&tagspec);
-	git_vector_free(&refs);
 	return error;
 }
 
diff --git a/src/transports/local.c b/src/transports/local.c
index 3163d2eac..4502f0202 100644
--- a/src/transports/local.c
+++ b/src/transports/local.c
@@ -213,21 +213,17 @@ static int local_connect(
 	return 0;
 }
 
-static int local_ls(git_transport *transport, git_headlist_cb list_cb, void *payload)
+static int local_ls(const git_remote_head ***out, size_t *size, git_transport *transport)
 {
 	transport_local *t = (transport_local *)transport;
-	unsigned int i;
-	git_remote_head *head = NULL;
 
 	if (!t->have_refs) {
 		giterr_set(GITERR_NET, "The transport has not yet loaded the refs");
 		return -1;
 	}
 
-	git_vector_foreach(&t->refs, i, head) {
-		if (list_cb(head, payload))
-			return GIT_EUSER;
-	}
+	*out = (const git_remote_head **) t->refs.contents;
+	*size = t->refs.length;
 
 	return 0;
 }
diff --git a/src/transports/smart.c b/src/transports/smart.c
index a681d5f40..53f880583 100644
--- a/src/transports/smart.c
+++ b/src/transports/smart.c
@@ -74,6 +74,7 @@ static int git_smart__connect(
 	transport_smart *t = (transport_smart *)transport;
 	git_smart_subtransport_stream *stream;
 	int error;
+	size_t i;
 	git_pkt *pkt;
 	git_pkt_ref *first;
 	git_smart_service_t service;
@@ -140,6 +141,16 @@ static int git_smart__connect(
 		git_pkt_free((git_pkt *)first);
 	}
 
+	/* Keep a list of heads for _ls */
+	git_vector_foreach(&t->refs, i, pkt) {
+		git_pkt_ref *ref = (git_pkt_ref *) pkt;
+		if (pkt->type != GIT_PKT_REF)
+			continue;
+
+		if (git_vector_insert(&t->heads, &ref->head) < 0)
+			return -1;
+	}
+
 	if (t->rpc && git_smart__reset_stream(t, false) < 0)
 		return -1;
 
@@ -149,28 +160,17 @@ static int git_smart__connect(
 	return 0;
 }
 
-static int git_smart__ls(git_transport *transport, git_headlist_cb list_cb, void *payload)
+static int git_smart__ls(const git_remote_head ***out, size_t *size, git_transport *transport)
 {
 	transport_smart *t = (transport_smart *)transport;
-	unsigned int i;
-	git_pkt *p = NULL;
 
 	if (!t->have_refs) {
 		giterr_set(GITERR_NET, "The transport has not yet loaded the refs");
 		return -1;
 	}
 
-	git_vector_foreach(&t->refs, i, p) {
-		git_pkt_ref *pkt = NULL;
-
-		if (p->type != GIT_PKT_REF)
-			continue;
-
-		pkt = (git_pkt_ref *)p;
-
-		if (list_cb(&pkt->head, payload))
-			return GIT_EUSER;
-	}
+	*out = (const git_remote_head **) t->heads.contents;
+	*size = t->heads.length;
 
 	return 0;
 }
@@ -293,6 +293,7 @@ static void git_smart__free(git_transport *transport)
 	/* Free the subtransport */
 	t->wrapped->free(t->wrapped);
 
+	git_vector_free(&t->heads);
 	git_vector_foreach(refs, i, p)
 		git_pkt_free(p);
 
@@ -340,6 +341,11 @@ int git_transport_smart(git_transport **out, git_remote *owner, void *param)
 		return -1;
 	}
 
+	if (git_vector_init(&t->heads, 16, ref_name_cmp) < 0) {
+		git__free(t);
+		return -1;
+	}
+
 	if (definition->callback(&t->wrapped, &t->parent) < 0) {
 		git__free(t);
 		return -1;
diff --git a/src/transports/smart.h b/src/transports/smart.h
index 5232e54de..b46a798a4 100644
--- a/src/transports/smart.h
+++ b/src/transports/smart.h
@@ -140,6 +140,7 @@ typedef struct {
 	git_smart_subtransport_stream *current_stream;
 	transport_smart_caps caps;
 	git_vector refs;
+	git_vector heads;
 	git_vector common;
 	git_atomic cancelled;
 	packetsize_cb packetsize_cb;
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index ce3f115ee..4fe7c0d09 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -32,6 +32,7 @@ int git_smart__store_refs(transport_smart *t, int flushes)
 	/* Clear existing refs in case git_remote_connect() is called again
 	 * after git_remote_disconnect().
 	 */
+	git_vector_clear(&t->heads);
 	git_vector_foreach(refs, i, ref) {
 		git__free(ref->head.name);
 		git__free(ref);