[v2,07/14] transport: stop needlessly copying bundle header references

Commit Message

Ævar Arnfjörð Bjarmason March 4, 2022, 6:32 p.m. UTC

Amend the logic added in fddf2ebe388 (transport: teach all vtables to
allow fetch first, 2019-08-21) and save ourselves pointless work in
fetch_refs_from_bundle().

The fetch_refs_from_bundle() caller doesn't care about the "struct
ref *result" return value of get_refs_from_bundle(), and doesn't need
any of the work we were doing in looping over the
"data->header.references" in get_refs_from_bundle().

So this change saves us work, and also fixes a memory leak that we had
when called from fetch_refs_from_bundle(). The other caller of
get_refs_from_bundle() is the "get_refs_list" member we set up for the
"struct transport_vtable bundle_vtable". That caller does care about
the "struct ref *result" return value.

Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
---
 transport.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

Comments

Derrick Stolee March 4, 2022, 7:10 p.m. UTC | #1

On 3/4/2022 1:32 PM, Ævar Arnfjörð Bjarmason wrote:
> Amend the logic added in fddf2ebe388 (transport: teach all vtables to
> allow fetch first, 2019-08-21) and save ourselves pointless work in
> fetch_refs_from_bundle().

> -static struct ref *get_refs_from_bundle(struct transport *transport,
> -					int for_push,
> -					struct transport_ls_refs_options *transport_options)
> +static void get_refs_from_bundle_inner(struct transport *transport)
>  {
>  	struct bundle_transport_data *data = transport->data;
> -	struct ref *result = NULL;
> -	int i;
> -
> -	if (for_push)
> -		return NULL;
>  
>  	data->get_refs_from_bundle_called = 1;

The inclusion of this line dramatically changed the shape of the
diff (different areas got selected as adds and deletes) so the
range-diff looked strange, but the end result is good.
  
Thanks,
-Stolee

Patch

diff --git a/transport.c b/transport.c
index 253d6671b1f..70e9840a90e 100644
--- a/transport.c
+++ b/transport.c
@@ -125,16 +125,9 @@  struct bundle_transport_data {
 	unsigned get_refs_from_bundle_called : 1;
 };
 
-static struct ref *get_refs_from_bundle(struct transport *transport,
-					int for_push,
-					struct transport_ls_refs_options *transport_options)
+static void get_refs_from_bundle_inner(struct transport *transport)
 {
 	struct bundle_transport_data *data = transport->data;
-	struct ref *result = NULL;
-	int i;
-
-	if (for_push)
-		return NULL;
 
 	data->get_refs_from_bundle_called = 1;
 
@@ -145,6 +138,20 @@  static struct ref *get_refs_from_bundle(struct transport *transport,
 		die(_("could not read bundle '%s'"), transport->url);
 
 	transport->hash_algo = data->header.hash_algo;
+}
+
+static struct ref *get_refs_from_bundle(struct transport *transport,
+					int for_push,
+					struct transport_ls_refs_options *transport_options)
+{
+	struct bundle_transport_data *data = transport->data;
+	struct ref *result = NULL;
+	int i;
+
+	if (for_push)
+		return NULL;
+
+	get_refs_from_bundle_inner(transport);
 
 	for (i = 0; i < data->header.references.nr; i++) {
 		struct string_list_item *e = data->header.references.items + i;
@@ -169,7 +176,7 @@  static int fetch_refs_from_bundle(struct transport *transport,
 		strvec_push(&extra_index_pack_args, "-v");
 
 	if (!data->get_refs_from_bundle_called)
-		get_refs_from_bundle(transport, 0, NULL);
+		get_refs_from_bundle_inner(transport);
 	ret = unbundle(the_repository, &data->header, data->fd,
 		       &extra_index_pack_args);
 	transport->hash_algo = data->header.hash_algo;