daemon: free listen_addr before returning

We build up a string list of listen addresses from the command-line
arguments, but never free it. This causes t5811 to complain of a leak
(though curiously it seems to do so only when compiled with gcc, not
with clang).

To handle this correctly, we have to do a little refactoring:

  - there are two exit points from the main function, depending on
    whether we are entering the main loop or serving a single client
    (since rather than a traditional fork model, we re-exec ourselves
    with the extra "--serve" argument to accommodate Windows).

    We don't need --listen at all in the --serve case, of course, but it
    is passed along by the parent daemon, which simply copies all of the
    command-line options it got.

  - we just "return serve()" to run the main loop, giving us no chance
    to do any cleanup

So let's use a "ret" variable to store the return code, and give
ourselves a single exit point at the end. That gives us one place to do
cleanup.

Note that this code also uses the "use a no-dup string-list, but
allocate strings we add to it" trick, meaning string_list_clear() will
not realize it should free them. We can fix this by switching to a "dup"
string-list, but using the "append_nodup" function to add to it (this is
preferable to tweaking the strdup_strings flag before clearing, as it
puts all the subtle memory-ownership code together).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/daemon.c b/daemon.c
index f5e597114b671fc79cd0acf63c63e5ac427af8bf..17d331b2f38465e295c019e534eff371ef3e6e49 100644 (file)
--- a/daemon.c
+++ b/daemon.c
@@ -1243,19 +1243,20 @@ static int serve(struct string_list *listen_addr, int listen_port,
 int cmd_main(int argc, const char **argv)
 {
        int listen_port = 0;
-       struct string_list listen_addr = STRING_LIST_INIT_NODUP;
+       struct string_list listen_addr = STRING_LIST_INIT_DUP;
        int serve_mode = 0, inetd_mode = 0;
        const char *pid_file = NULL, *user_name = NULL, *group_name = NULL;
        int detach = 0;
        struct credentials *cred = NULL;
        int i;
+       int ret;
 
        for (i = 1; i < argc; i++) {
                const char *arg = argv[i];
                const char *v;
 
                if (skip_prefix(arg, "--listen=", &v)) {
-                       string_list_append(&listen_addr, xstrdup_tolower(v));
+                       string_list_append_nodup(&listen_addr, xstrdup_tolower(v));
                        continue;
                }
                if (skip_prefix(arg, "--port=", &v)) {
@@ -1437,22 +1438,26 @@ int cmd_main(int argc, const char **argv)
                        die_errno("failed to redirect stderr to /dev/null");
        }
 
-       if (inetd_mode || serve_mode)
-               return execute();
+       if (inetd_mode || serve_mode) {
+               ret = execute();
+       } else {
+               if (detach) {
+                       if (daemonize())
+                               die("--detach not supported on this platform");
+               }
 
-       if (detach) {
-               if (daemonize())
-                       die("--detach not supported on this platform");
-       }
+               if (pid_file)
+                       write_file(pid_file, "%"PRIuMAX, (uintmax_t) getpid());
 
-       if (pid_file)
-               write_file(pid_file, "%"PRIuMAX, (uintmax_t) getpid());
+               /* prepare argv for serving-processes */
+               strvec_push(&cld_argv, argv[0]); /* git-daemon */
+               strvec_push(&cld_argv, "--serve");
+               for (i = 1; i < argc; ++i)
+                       strvec_push(&cld_argv, argv[i]);
 
-       /* prepare argv for serving-processes */
-       strvec_push(&cld_argv, argv[0]); /* git-daemon */
-       strvec_push(&cld_argv, "--serve");
-       for (i = 1; i < argc; ++i)
-               strvec_push(&cld_argv, argv[i]);
+               ret = serve(&listen_addr, listen_port, cred);
+       }
 
-       return serve(&listen_addr, listen_port, cred);
+       string_list_clear(&listen_addr, 0);
+       return ret;
 }

diff --git a/t/t5811-proto-disable-git.sh b/t/t5811-proto-disable-git.sh
index 8ac6b2a1d0a286cac120c751e5e3038e465dc074..ed773e7432694b7ccc05a4d09084f238c9ff4b42 100755 (executable)
--- a/t/t5811-proto-disable-git.sh
+++ b/t/t5811-proto-disable-git.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test disabling of git-over-tcp in clone/fetch'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-proto-disable.sh"
 . "$TEST_DIRECTORY/lib-git-daemon.sh"