transport-helper: fix leaking helper name

When initializing the transport helper in `transport_get()`, we
allocate the name of the helper. We neither end up transferring
ownership of the name, nor do we free it. The associated memory thus
leaks.

Fix this memory leak by freeing the string at the calling side in
`transport_get()`. `transport_helper_init()` now creates its own copy of
the string and thus can free it as required.

An alterantive way to fix this would be to transfer ownership of the
string passed into `transport_helper_init()`, which would avoid the call
to xstrdup(1). But it does make for a more surprising calling convention
as we do not typically transfer ownership of strings like this.

Mark now-passing tests as leak free.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t0611-reftable-httpd.sh b/t/t0611-reftable-httpd.sh
index 5e05b9c..2805995 100755 (executable)
--- a/t/t0611-reftable-httpd.sh
+++ b/t/t0611-reftable-httpd.sh
@@ -2,6 +2,7 @@
 
 test_description='reftable HTTPD tests'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-httpd.sh
 

diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index 5d5caa3..4af796d 100755 (executable)
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -2,6 +2,7 @@
 
 test_description='test http auth header and credential helper interop'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-httpd.sh
 

diff --git a/t/t5564-http-proxy.sh b/t/t5564-http-proxy.sh
index 9da5134..bb35b87 100755 (executable)
--- a/t/t5564-http-proxy.sh
+++ b/t/t5564-http-proxy.sh
@@ -2,6 +2,7 @@
 
 test_description="test fetching through http proxy"
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-httpd.sh
 

diff --git a/t/t5581-http-curl-verbose.sh b/t/t5581-http-curl-verbose.sh
index cded79c..724f610 100755 (executable)
--- a/t/t5581-http-curl-verbose.sh
+++ b/t/t5581-http-curl-verbose.sh
@@ -4,6 +4,7 @@ test_description='test GIT_CURL_VERBOSE'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd

diff --git a/transport-helper.c b/transport-helper.c
index 780fcaf..9820947 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -22,7 +22,7 @@
 static int debug;
 
 struct helper_data {
-       const char *name;
+       char *name;
        struct child_process *helper;
        FILE *out;
        unsigned fetch : 1,
@@ -111,6 +111,7 @@ static void do_take_over(struct transport *transport)
        data = (struct helper_data *)transport->data;
        transport_take_over(transport, data->helper);
        fclose(data->out);
+       free(data->name);
        free(data);
 }
 
@@ -253,6 +254,7 @@ static int disconnect_helper(struct transport *transport)
                close(data->helper->out);
                fclose(data->out);
                res = finish_command(data->helper);
+               FREE_AND_NULL(data->name);
                FREE_AND_NULL(data->helper);
        }
        return res;
@@ -1297,7 +1299,7 @@ static struct transport_vtable vtable = {
 int transport_helper_init(struct transport *transport, const char *name)
 {
        struct helper_data *data = xcalloc(1, sizeof(*data));
-       data->name = name;
+       data->name = xstrdup(name);
 
        transport_check_allowed(name);
 

diff --git a/transport.c b/transport.c
index 0ad04b7..83ddea8 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -1176,6 +1176,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
                int len = external_specification_len(url);
                char *handler = xmemdupz(url, len);
                transport_helper_init(ret, handler);
+               free(handler);
        }
 
        if (ret->smart_options) {