Fix notification environment variable setting.
authorDavin McCall <davmac@davmac.org>
Sat, 29 Jun 2019 10:20:55 +0000 (11:20 +0100)
committerDavin McCall <davmac@davmac.org>
Fri, 5 Jul 2019 08:33:19 +0000 (18:33 +1000)
Refactor start_ps_process to take a single struct argument, and fix a
bug in the process: the notification environment variable (where the
notification fd should be stored) was never passed through to the
service process.

src/baseproc-service.cc
src/includes/proc-service.h
src/run-child-proc.cc

index ba2227c017f27a3607b596f57ef571e304e3df3f..0bc9414f1d0f09b26f3445a03bafe775aa60f206 100644 (file)
@@ -155,8 +155,14 @@ bool base_process_service::start_ps_process(const std::vector<const char *> &cmd
         const char * working_dir_c = nullptr;
         if (! working_dir.empty()) working_dir_c = working_dir.c_str();
         after_fork(getpid());
-        run_child_proc(cmd.data(), working_dir_c, logfile, on_console, pipefd[1], control_socket[1],
-                socket_fd, notify_pipe[1], force_notification_fd, nullptr, run_as_uid, run_as_gid, rlimits);
+        run_proc_params run_params{cmd.data(), working_dir_c, logfile, pipefd[1], run_as_uid, run_as_gid, rlimits};
+        run_params.on_console = on_console;
+        run_params.csfd = control_socket[1];
+        run_params.socket_fd = socket_fd;
+        run_params.notify_fd = notify_pipe[1];
+        run_params.force_notify_fd = force_notification_fd;
+        run_params.notify_var = notification_var.c_str();
+        run_child_proc(run_params);
     }
     else {
         // Parent process
index fe36ea0fec76be68e6a7e9b457b04505b55f75ba..f3af5413f022afbe73829d036d3f7b199bf339fe 100644 (file)
@@ -25,6 +25,31 @@ struct service_rlimits
     service_rlimits(int id) : resource_id(id), soft_set(0), hard_set(0), limits({0,0}) { }
 };
 
+// Parameters for process execution
+struct run_proc_params
+{
+    const char * const *args; // program arguments including executable (args[0])
+    const char *working_dir;  // working directory
+    const char *logfile;      // log file or nullptr (stdout/stderr); must be valid if !on_console
+    bool on_console;          // whether to run on console
+    int wpipefd;              // pipe to which error status will be sent (if error occurs)
+    int csfd;                 // control socket fd (or -1); may be moved
+    int socket_fd;            // pre-opened socket fd (or -1); may be moved
+    int notify_fd;            // pipe for readiness notification message (or -1); may be moved
+    int force_notify_fd;      // if not -1, notification fd must be moved to this fd
+    const char *notify_var;   // environment variable name where notification fd will be stored, or nullptr
+    uid_t uid;
+    gid_t gid;
+    const std::vector<service_rlimits> &rlimits;
+
+    run_proc_params(const char * const *args, const char *working_dir, const char *logfile, int wpipefd,
+            uid_t uid, gid_t gid, const std::vector<service_rlimits> &rlimits)
+            : args(args), working_dir(working_dir), logfile(logfile), on_console(false), wpipefd(wpipefd),
+              csfd(-1), socket_fd(-1), notify_fd(-1), force_notify_fd(-1), notify_var(nullptr), uid(uid),
+              gid(gid), rlimits(rlimits)
+    { }
+};
+
 class base_process_service;
 
 // A timer for process restarting. Used to ensure a minimum delay between process restarts (and
@@ -145,26 +170,9 @@ class base_process_service : public service_record
     bool reserved_child_watch : 1;
     bool tracking_child : 1;  // whether we expect to see child process status
 
-    // Run a child process (call after forking). Note that some arguments specify file descriptors,
+    // Run a child process (call after forking). Note that some parameters specify file descriptors,
     // but in general file descriptors may be moved before the exec call.
-    // - args specifies the program arguments including the executable (argv[0])
-    // - working_dir specifies the working directory; may be null
-    // - logfile specifies the logfile (where stdout/stderr are directed)
-    // - on_console: if true, process is run with access to console
-    // - wpipefd: if the exec is unsuccessful, or another error occurs beforehand, the
-    //   error number (errno) is written to this file descriptor
-    // - csfd: the control socket fd; may be -1 to inhibit passing of control socket
-    // - socket_fd: the pre-opened socket file descriptor (may be -1)
-    // - notify_fd: the readiness notification fd; process should write to this descriptor when
-    //   is is ready
-    // - force_notify_fd: if not -1, specifies the file descriptor that notify_fd should be moved
-    //   to (via dup2 and close of the original).
-    // - notify_var: the name of an environment variable which will be set to contain the notification
-    //   fd
-    // - uid/gid: the identity to run the process as (may be both -1, otherwise both must be valid)
-    void run_child_proc(const char * const *args, const char *working_dir, const char *logfile,
-            bool on_console, int wpipefd, int csfd, int socket_fd, int notify_fd, int force_notify_fd,
-            const char *notify_var,uid_t uid, gid_t gid, const std::vector<service_rlimits> &rlimits) noexcept;
+    void run_child_proc(run_proc_params params) noexcept;
 
     // Launch the process with the given arguments, return true on success
     bool start_ps_process(const std::vector<const char *> &args, bool on_console) noexcept;
index 958f928fe3fd3a137ac93ac52c57ad33ffa9ebb4..837a2c9ee2181abfcd90531133423064fa337d22 100644 (file)
@@ -40,12 +40,21 @@ static int move_reserved_fd(int *fd, int min_fd)
     return new_fd;
 }
 
-void base_process_service::run_child_proc(const char * const *args, const char *working_dir,
-        const char *logfile, bool on_console, int wpipefd, int csfd, int socket_fd,
-        int notify_fd, int force_notify_fd, const char *notify_var,
-        uid_t uid, gid_t gid, const std::vector<service_rlimits> &rlimits) noexcept
+void base_process_service::run_child_proc(run_proc_params params) noexcept
 {
     // Child process. Must not risk throwing any uncaught exception from here until exit().
+    const char * const *args = params.args;
+    const char *working_dir = params.working_dir;
+    const char *logfile = params.logfile;
+    bool on_console = params.on_console;
+    int wpipefd = params.wpipefd;
+    int csfd = params.csfd;
+    int notify_fd = params.notify_fd;
+    int force_notify_fd = params.force_notify_fd;
+    const char *notify_var = params.notify_var;
+    uid_t uid = params.uid;
+    gid_t gid = params.gid;
+    const std::vector<service_rlimits> &rlimits = params.rlimits;
 
     // If the console already has a session leader, presumably it is us. On the other hand
     // if it has no session leader, and we don't create one, then control inputs such as