From 81dd60f8755591f8c2ae338806c8c5aaaad2c50f Mon Sep 17 00:00:00 2001
From: Donald Buczek <buczek@molgen.mpg.de>
Date: Wed, 10 Jan 2024 13:08:38 +0100
Subject: [PATCH] mxqd: Make setup_cronolog() into void function

Currently, setup_cronolog() returns on errors. However, in most error
paths, it leaks one or more file descriptors.  The leaked descriptors
triggers warnings in with the gcc 13 static analyzer.

As the only caller of this functions terminates the prohram on any error
anyway, don't fix the file descriptor leaks, but terminate right after
any error from inside the function.
---
 mxqd.c | 52 +++++++++++++++-------------------------------------
 1 file changed, 15 insertions(+), 37 deletions(-)

diff --git a/mxqd.c b/mxqd.c
index 30f2c3c..1a55545 100644
--- a/mxqd.c
+++ b/mxqd.c
@@ -158,9 +158,8 @@ static void cpuset_clear_running(cpu_set_t *running,cpu_set_t *job) {
 }
 
 /**********************************************************************/
-static int setup_cronolog(char *cronolog, char *logdir, char *rellink, char *relformat)
+static void setup_cronolog(char *cronolog, char *logdir, char *rellink, char *relformat)
 {
-    int res;
     int pipe_fd[2];
     int pid;
     _mx_cleanup_free_ char *link = NULL;
@@ -174,51 +173,34 @@ static int setup_cronolog(char *cronolog, char *logdir, char *rellink, char *rel
         format = strdup(relformat);
     }
 
-    if (!link || !format) {
-        mx_log_err("can't allocate filenames: (%m)");
-        return 0;
-    }
+    if (!link || !format)
+        mx_die("can't allocate filenames: (%m)");
 
-    res = pipe(pipe_fd);
-    if (res == -1) {
-        mx_log_err("can't create pipe for cronolog: (%m)");
-        return 0;
-    }
+    if (pipe(pipe_fd) == -1)
+        mx_die("can't create pipe for cronolog: (%m)");
 
     pid = fork();
     if (pid < 0) {
-        mx_log_err("cronolog fork failed: %m");
-        return 0;
+        mx_die("cronolog fork failed: %m");
     } else if(pid == 0) {
-        res = dup2(pipe_fd[0], STDIN_FILENO);
-        if (res == -1) {
-            mx_log_err("dup2(fh=%d, %d) for cronolog stdin failed (%m)", pipe_fd[0], STDIN_FILENO);
-            return 0;
-        }
+        if (dup2(pipe_fd[0], STDIN_FILENO) == -1)
+            mx_die("dup2(fh=%d, %d) for cronolog stdin failed (%m)", pipe_fd[0], STDIN_FILENO);
         close(pipe_fd[0]);
         close(pipe_fd[1]);
 
         mxq_redirect_output("/dev/null", "/dev/null");
 
         execl(cronolog, cronolog, "--link", link, format, NULL);
-        mx_log_err("execl('%s', ...) failed (%m)", cronolog);
-        _exit(EX__MAX + 1);
+        mx_die("execl('%s', ...) failed (%m)", cronolog);
     }
 
-    res = dup2(pipe_fd[1], STDOUT_FILENO);
-    if (res == -1) {
-        mx_log_err("dup2(fh=%d, %d) for cronolog stdout failed (%m)", pipe_fd[0], STDOUT_FILENO);
-        return 0;
-    }
-    res = dup2(STDOUT_FILENO, STDERR_FILENO);
-    if (res == -1) {
-        mx_log_err("dup2(fh=%d, %d) for cronolog stderr failed (%m)", STDOUT_FILENO, STDERR_FILENO);
-        return 0;
-    }
+    if (dup2(pipe_fd[1], STDOUT_FILENO) == -1)
+        mx_die("dup2(fh=%d, %d) for cronolog stdout failed (%m)", pipe_fd[0], STDOUT_FILENO);
+    if (dup2(STDOUT_FILENO, STDERR_FILENO) == -1)
+        mx_die("dup2(fh=%d, %d) for cronolog stderr failed (%m)", STDOUT_FILENO, STDERR_FILENO);
+
     close(pipe_fd[0]);
     close(pipe_fd[1]);
-
-    return pid;
 }
 
 
@@ -723,11 +705,7 @@ static int server_init(struct mxq_server *server, int argc, char *argv[])
             mx_log_err("MAIN: can't write to '%s': %m", arg_logdir);
             return -EX_IOERR;
         }
-        res = setup_cronolog("/usr/sbin/cronolog", arg_logdir, "mxqd_log", "%Y/mxqd_log-%Y-%m");
-        if (!res) {
-            mx_log_err("MAIN: cronolog setup failed. exiting.");
-            return -EX_IOERR;
-        }
+        setup_cronolog("/usr/sbin/cronolog", arg_logdir, "mxqd_log", "%Y/mxqd_log-%Y-%m");
     }
 
     res = mx_mysql_initialize(&(server->mysql));