Skip to content

Commit

Permalink
mxqd: Make setup_cronolog() into void function
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
donald committed Jan 10, 2024
1 parent 673fa5e commit 0321629
Showing 1 changed file with 15 additions and 37 deletions.
52 changes: 15 additions & 37 deletions mxqd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}


Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 0321629

Please sign in to comment.