ls-remote & transport API: release "struct transport_ls_refs_options"

Fix a memory leak in codepaths that use the "struct
transport_ls_refs_options" API. Since the introduction of the struct
in 39835409d10 (connect, transport: encapsulate arg in struct,
2021-02-05) the caller has been responsible for freeing it.

That commit in turn migrated code originally added in
402c47d9391 (clone: send ref-prefixes when using protocol v2,
2018-07-20) and b4be74105fe (ls-remote: pass ref prefixes when
requesting a remote's refs, 2018-03-15). Only some of those codepaths
were releasing the allocated resources of the struct, now all of them
will.

Mark the "t/t5511-refspec.sh" test as passing when git is compiled
with SANITIZE=leak. They'll now be listed as running under the
"GIT_TEST_PASSING_SANITIZE_LEAK=true" test mode (the "linux-leaks" CI
target). Previously 24/47 tests would fail.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/clone.c b/builtin/clone.c
index 727e16e0aea435a1fe244c381d8bb7670e8e7a5a..8564e5f603fdff9a663f4e8e61ad2ca0316893ef 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1233,7 +1233,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
        }
        else {
                const char *branch;
-               char *ref;
+               const char *ref;
+               char *ref_free = NULL;
 
                if (option_branch)
                        die(_("Remote branch %s not found in upstream %s"),
@@ -1250,17 +1251,16 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
                    skip_prefix(transport_ls_refs_options.unborn_head_target,
                                "refs/heads/", &branch)) {
                        ref = transport_ls_refs_options.unborn_head_target;
-                       transport_ls_refs_options.unborn_head_target = NULL;
                        create_symref("HEAD", ref, reflog_msg.buf);
                } else {
                        branch = git_default_branch_name(0);
-                       ref = xstrfmt("refs/heads/%s", branch);
+                       ref_free = xstrfmt("refs/heads/%s", branch);
+                       ref = ref_free;
                }
 
                if (!option_bare)
                        install_branch_config(0, branch, remote_name, ref);
-
-               free(ref);
+               free(ref_free);
        }
 
        write_refspec_config(src_ref_prefix, our_head_points_at,
@@ -1312,7 +1312,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
        UNLEAK(repo);
        junk_mode = JUNK_LEAVE_ALL;
 
-       strvec_clear(&transport_ls_refs_options.ref_prefixes);
-       free(transport_ls_refs_options.unborn_head_target);
+       transport_ls_refs_options_release(&transport_ls_refs_options);
        return err;
 }

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5f06b21f8e97c5459fdb558302c5b9b95e99eee8..a3ffab727eba29302ab46a32a6cb6562b6fa0ee4 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1593,7 +1593,7 @@ static int do_fetch(struct transport *transport,
        } else
                remote_refs = NULL;
 
-       strvec_clear(&transport_ls_refs_options.ref_prefixes);
+       transport_ls_refs_options_release(&transport_ls_refs_options);
 
        ref_map = get_ref_map(transport->remote, remote_refs, rs,
                              tags, &autotags);

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 44448fa61d168d7b846f5e241b58e0e6cce9e128..d856085e9414a2bea070cbe9c84268c66158c33a 100644 (file)
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -155,6 +155,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 
        ref_array_clear(&ref_array);
        if (transport_disconnect(transport))
-               return 1;
+               status = 1;
+       transport_ls_refs_options_release(&transport_options);
        return status;
 }

diff --git a/connect.c b/connect.c
index eaf7d6d26187f7265e507bca6ba8d6bf5110f26b..afc79a6236e8d5fa7d4f6092c0e89d45e46cba41 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -379,7 +379,7 @@ struct ref **get_remote_heads(struct packet_reader *reader,
 
 /* Returns 1 when a valid ref has been added to `list`, 0 otherwise */
 static int process_ref_v2(struct packet_reader *reader, struct ref ***list,
-                         char **unborn_head_target)
+                         const char **unborn_head_target)
 {
        int ret = 1;
        int i = 0;
@@ -483,7 +483,7 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
        const char *hash_name;
        struct strvec *ref_prefixes = transport_options ?
                &transport_options->ref_prefixes : NULL;
-       char **unborn_head_target = transport_options ?
+       const char **unborn_head_target = transport_options ?
                &transport_options->unborn_head_target : NULL;
        *list = NULL;
 

diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh
index be025b90f989f68d2b60b96ee7e29b134b557f6f..fc55681a3f2cb5f1b27e6b939142e4c7693e043b 100755 (executable)
--- a/t/t5511-refspec.sh
+++ b/t/t5511-refspec.sh
@@ -2,6 +2,7 @@
 
 test_description='refspec parsing'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_refspec () {

diff --git a/transport.c b/transport.c
index 2a3e32415455baf09f165837cb169e31ab6876f5..253d6671b1f1edb889525b0ed8a2bb2769540b33 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -1292,7 +1292,7 @@ int transport_push(struct repository *r,
                                                               &transport_options);
                trace2_region_leave("transport_push", "get_refs_list", r);
 
-               strvec_clear(&transport_options.ref_prefixes);
+               transport_ls_refs_options_release(&transport_options);
 
                if (flags & TRANSPORT_PUSH_ALL)
                        match_flags |= MATCH_REFS_ALL;
@@ -1420,6 +1420,12 @@ const struct ref *transport_get_remote_refs(struct transport *transport,
        return transport->remote_refs;
 }
 
+void transport_ls_refs_options_release(struct transport_ls_refs_options *opts)
+{
+       strvec_clear(&opts->ref_prefixes);
+       free((char *)opts->unborn_head_target);
+}
+
 int transport_fetch_refs(struct transport *transport, struct ref *refs)
 {
        int rc;

diff --git a/transport.h b/transport.h
index 3f16e50c1965db222173a2330b12c26aee3d6d25..a0bc6a1e9eba8f7e3c034abc6ba5e0d685782959 100644 (file)
--- a/transport.h
+++ b/transport.h
@@ -257,15 +257,19 @@ struct transport_ls_refs_options {
        /*
         * If unborn_head_target is not NULL, and the remote reports HEAD as
         * pointing to an unborn branch, transport_get_remote_refs() stores the
-        * unborn branch in unborn_head_target. It should be freed by the
-        * caller.
+        * unborn branch in unborn_head_target.
         */
-       char *unborn_head_target;
+       const char *unborn_head_target;
 };
 #define TRANSPORT_LS_REFS_OPTIONS_INIT { \
        .ref_prefixes = STRVEC_INIT, \
 }
 
+/**
+ * Release the "struct transport_ls_refs_options".
+ */
+void transport_ls_refs_options_release(struct transport_ls_refs_options *opts);
+
 /*
  * Retrieve refs from a remote.
  */