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>

Jeff King committed Oct 5, 2023 at 17:33 UTC badf2fe1c31f939cac5ea229bba8de273af132d9
2 files changed +23 -16
daemon.c
+21 -16
@@ -1243,19 +1243,20 @@ static int serve(struct string_list *listen_addr, int listen_port,
1243 int cmd_main(int argc, const char **argv)
1244 {
1245 int listen_port = 0;
1246 - struct string_list listen_addr = STRING_LIST_INIT_NODUP;
1246 + struct string_list listen_addr = STRING_LIST_INIT_DUP;
1247 int serve_mode = 0, inetd_mode = 0;
1248 const char *pid_file = NULL, *user_name = NULL, *group_name = NULL;
1249 int detach = 0;
1250 struct credentials *cred = NULL;
1251 int i;
1252 + int ret;
1253
1254 for (i = 1; i < argc; i++) {
1255 const char *arg = argv[i];
1256 const char *v;
1257
1258 if (skip_prefix(arg, "--listen=", &v)) {
1258 - string_list_append(&listen_addr, xstrdup_tolower(v));
1259 + string_list_append_nodup(&listen_addr, xstrdup_tolower(v));
1260 continue;
1261 }
1262 if (skip_prefix(arg, "--port=", &v)) {
@@ -1437,22 +1438,26 @@ int cmd_main(int argc, const char **argv)
1438 die_errno("failed to redirect stderr to /dev/null");
1439 }
1440
1440 - if (inetd_mode || serve_mode)
1441 - return execute();
1441 + if (inetd_mode || serve_mode) {
1442 + ret = execute();
1443 + } else {
1444 + if (detach) {
1445 + if (daemonize())
1446 + die("--detach not supported on this platform");
1447 + }
1448
1443 - if (detach) {
1444 - if (daemonize())
1445 - die("--detach not supported on this platform");
1446 - }
1449 + if (pid_file)
1450 + write_file(pid_file, "%"PRIuMAX, (uintmax_t) getpid());
1451
1448 - if (pid_file)
1449 - write_file(pid_file, "%"PRIuMAX, (uintmax_t) getpid());
1452 + /* prepare argv for serving-processes */
1453 + strvec_push(&cld_argv, argv[0]); /* git-daemon */
1454 + strvec_push(&cld_argv, "--serve");
1455 + for (i = 1; i < argc; ++i)
1456 + strvec_push(&cld_argv, argv[i]);
1457
1451 - /* prepare argv for serving-processes */
1452 - strvec_push(&cld_argv, argv[0]); /* git-daemon */
1453 - strvec_push(&cld_argv, "--serve");
1454 - for (i = 1; i < argc; ++i)
1455 - strvec_push(&cld_argv, argv[i]);
1458 + ret = serve(&listen_addr, listen_port, cred);
1459 + }
1460
1457 - return serve(&listen_addr, listen_port, cred);
1461 + string_list_clear(&listen_addr, 0);
1462 + return ret;
1463 }
t/t5811-proto-disable-git.sh
+2
@@ -1,6 +1,8 @@
1 #!/bin/sh
2
3 test_description='test disabling of git-over-tcp in clone/fetch'
4 +
5 +TEST_PASSES_SANITIZE_LEAK=true
6 . ./test-lib.sh
7 . "$TEST_DIRECTORY/lib-proto-disable.sh"
8 . "$TEST_DIRECTORY/lib-git-daemon.sh"