From 03216294063d85a6b1e7f73eaed61099217a5d77 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));