core: remove the redundancy of 'n_fds' and 'n_storage_fds' in ExecParameters struct

'n_fds' field in the ExecParameters structure was counting the total number of
file descriptors to be passed to a unit.

This counter also includes the number of passed socket fds which is counted by
'n_socket_fds' already.

This patch removes that redundancy by replacing 'n_fds' with
'n_storage_fds'. The new field only counts the fds passed via the storage store
mechanism.  That way each fd is counted at one place only.

Subsequently the patch makes sure to fix code that used 'n_fds' and also wanted
to iterate through all of them by explicitly adding 'n_socket_fds' + 'n_storage_fds'.

Suggested by Lennart.

diff --git a/src/core/execute.c b/src/core/execute.c
index 7d9fc037b923d39c5c1cd56c17daa81afb6e04b1..550a8cfd39673523a5558965ccb219bef385ff6f 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -157,10 +157,11 @@ static int shift_fds(int fds[], unsigned n_fds) {
         return 0;
 }
 
-static int flags_fds(const int fds[], unsigned n_fds, unsigned n_socket_fds, bool nonblock) {
-        unsigned i;
+static int flags_fds(const int fds[], unsigned n_storage_fds, unsigned n_socket_fds, bool nonblock) {
+        unsigned i, n_fds;
         int r;
 
+        n_fds = n_storage_fds + n_socket_fds;
         if (n_fds <= 0)
                 return 0;
 
@@ -2244,7 +2245,8 @@ static int exec_child(
                 char **argv,
                 int socket_fd,
                 int named_iofds[3],
-                int *fds, unsigned n_fds,
+                int *fds,
+                unsigned n_storage_fds,
                 unsigned n_socket_fds,
                 char **files_env,
                 int user_lookup_fd,
@@ -2262,6 +2264,7 @@ static int exec_child(
         uid_t uid = UID_INVALID;
         gid_t gid = GID_INVALID;
         int i, r, ngids = 0;
+        unsigned n_fds;
 
         assert(unit);
         assert(command);
@@ -2302,6 +2305,7 @@ static int exec_child(
 
         log_forget_fds();
 
+        n_fds = n_storage_fds + n_socket_fds;
         r = close_remaining_fds(params, runtime, dcreds, user_lookup_fd, socket_fd, fds, n_fds);
         if (r < 0) {
                 *exit_status = EXIT_FDS;
@@ -2673,7 +2677,7 @@ static int exec_child(
         if (r >= 0)
                 r = shift_fds(fds, n_fds);
         if (r >= 0)
-                r = flags_fds(fds, n_fds, n_socket_fds, context->non_blocking);
+                r = flags_fds(fds, n_storage_fds, n_socket_fds, context->non_blocking);
         if (r < 0) {
                 *exit_status = EXIT_FDS;
                 return r;
@@ -2914,7 +2918,7 @@ int exec_spawn(Unit *unit,
 
         _cleanup_strv_free_ char **files_env = NULL;
         int *fds = NULL;
-        unsigned n_fds = 0, n_socket_fds = 0;
+        unsigned n_storage_fds = 0, n_socket_fds = 0;
         _cleanup_free_ char *line = NULL;
         int socket_fd, r;
         int named_iofds[3] = { -1, -1, -1 };
@@ -2926,18 +2930,18 @@ int exec_spawn(Unit *unit,
         assert(context);
         assert(ret);
         assert(params);
-        assert(params->fds || params->n_fds <= 0);
+        assert(params->fds || (params->n_storage_fds + params->n_socket_fds <= 0));
 
         if (context->std_input == EXEC_INPUT_SOCKET ||
             context->std_output == EXEC_OUTPUT_SOCKET ||
             context->std_error == EXEC_OUTPUT_SOCKET) {
 
-                if (params->n_fds > 1) {
+                if (params->n_socket_fds > 1) {
                         log_unit_error(unit, "Got more than one socket.");
                         return -EINVAL;
                 }
 
-                if (params->n_fds == 0) {
+                if (params->n_socket_fds == 0) {
                         log_unit_error(unit, "Got no socket.");
                         return -EINVAL;
                 }
@@ -2946,7 +2950,7 @@ int exec_spawn(Unit *unit,
         } else {
                 socket_fd = -1;
                 fds = params->fds;
-                n_fds = params->n_fds;
+                n_storage_fds = params->n_storage_fds;
                 n_socket_fds = params->n_socket_fds;
         }
 
@@ -2985,7 +2989,8 @@ int exec_spawn(Unit *unit,
                                argv,
                                socket_fd,
                                named_iofds,
-                               fds, n_fds,
+                               fds,
+                               n_storage_fds,
                                n_socket_fds,
                                files_env,
                                unit->manager->user_lookup_fds[1],
@@ -3195,6 +3200,7 @@ const char* exec_context_fdname(const ExecContext *c, int fd_index) {
 int exec_context_named_iofds(Unit *unit, const ExecContext *c, const ExecParameters *p, int named_iofds[3]) {
         unsigned i, targets;
         const char* stdio_fdname[3];
+        unsigned n_fds;
 
         assert(c);
         assert(p);
@@ -3206,7 +3212,9 @@ int exec_context_named_iofds(Unit *unit, const ExecContext *c, const ExecParamet
         for (i = 0; i < 3; i++)
                 stdio_fdname[i] = exec_context_fdname(c, i);
 
-        for (i = 0; i < p->n_fds && targets > 0; i++)
+        n_fds = p->n_storage_fds + p->n_socket_fds;
+
+        for (i = 0; i < n_fds  && targets > 0; i++)
                 if (named_iofds[STDIN_FILENO] < 0 &&
                     c->std_input == EXEC_INPUT_NAMED_FD &&
                     stdio_fdname[STDIN_FILENO] &&

diff --git a/src/core/execute.h b/src/core/execute.h
index dee7af14239c74f554c7c9d867c25e405e8581f1..136319359c048ae0c5591c78a5a6e6c2ea55cbf9 100644 (file)
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -246,7 +246,7 @@ struct ExecParameters {
 
         int *fds;
         char **fd_names;
-        unsigned n_fds;
+        unsigned n_storage_fds;
         unsigned n_socket_fds;
 
         ExecFlags flags;

diff --git a/src/core/service.c b/src/core/service.c
index 370a6316e7f8235f4d9a5b92c95fdfbd6738e30d..df7f1f3053a815f3613164d8f2744226475992e6 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1068,11 +1068,16 @@ static int service_coldplug(Unit *u) {
         return 0;
 }
 
-static int service_collect_fds(Service *s, int **fds, char ***fd_names, unsigned *n_socket_fds) {
+static int service_collect_fds(Service *s,
+                               int **fds,
+                               char ***fd_names,
+                               unsigned *n_storage_fds,
+                               unsigned *n_socket_fds) {
+
         _cleanup_strv_free_ char **rfd_names = NULL;
         _cleanup_free_ int *rfds = NULL;
-        unsigned rn_socket_fds = 0;
-        int rn_fds = 0,  r;
+        unsigned rn_socket_fds = 0, rn_storage_fds = 0;
+        int r;
 
         assert(s);
         assert(fds);
@@ -1092,7 +1097,7 @@ static int service_collect_fds(Service *s, int **fds, char ***fd_names, unsigned
                 if (!rfd_names)
                         return -ENOMEM;
 
-                rn_fds = 1;
+                rn_socket_fds = 1;
         } else {
                 Iterator i;
                 Unit *u;
@@ -1118,20 +1123,20 @@ static int service_collect_fds(Service *s, int **fds, char ***fd_names, unsigned
 
                         if (!rfds) {
                                 rfds = cfds;
-                                rn_fds = cn_fds;
+                                rn_socket_fds = cn_fds;
 
                                 cfds = NULL;
                         } else {
                                 int *t;
 
-                                t = realloc(rfds, (rn_fds + cn_fds) * sizeof(int));
+                                t = realloc(rfds, (rn_socket_fds + cn_fds) * sizeof(int));
                                 if (!t)
                                         return -ENOMEM;
 
-                                memcpy(t + rn_fds, cfds, cn_fds * sizeof(int));
+                                memcpy(t + rn_socket_fds, cfds, cn_fds * sizeof(int));
 
                                 rfds = t;
-                                rn_fds += cn_fds;
+                                rn_socket_fds += cn_fds;
                         }
 
                         r = strv_extend_n(&rfd_names, socket_fdname(sock), cn_fds);
@@ -1140,45 +1145,47 @@ static int service_collect_fds(Service *s, int **fds, char ***fd_names, unsigned
                 }
         }
 
-        rn_socket_fds = rn_fds;
-
         if (s->n_fd_store > 0) {
                 ServiceFDStore *fs;
+                unsigned n_fds;
                 char **nl;
                 int *t;
 
-                t = realloc(rfds, (rn_fds + s->n_fd_store) * sizeof(int));
+                t = realloc(rfds, (rn_socket_fds + s->n_fd_store) * sizeof(int));
                 if (!t)
                         return -ENOMEM;
 
                 rfds = t;
 
-                nl = realloc(rfd_names, (rn_fds + s->n_fd_store + 1) * sizeof(char*));
+                nl = realloc(rfd_names, (rn_socket_fds + s->n_fd_store + 1) * sizeof(char*));
                 if (!nl)
                         return -ENOMEM;
 
                 rfd_names = nl;
+                n_fds = rn_socket_fds;
 
                 LIST_FOREACH(fd_store, fs, s->fd_store) {
-                        rfds[rn_fds] = fs->fd;
-                        rfd_names[rn_fds] = strdup(strempty(fs->fdname));
-                        if (!rfd_names[rn_fds])
+                        rfds[n_fds] = fs->fd;
+                        rfd_names[n_fds] = strdup(strempty(fs->fdname));
+                        if (!rfd_names[n_fds])
                                 return -ENOMEM;
 
-                        rn_fds++;
+                        rn_storage_fds++;
+                        n_fds++;
                 }
 
-                rfd_names[rn_fds] = NULL;
+                rfd_names[n_fds] = NULL;
         }
 
         *fds = rfds;
         *fd_names = rfd_names;
         *n_socket_fds = rn_socket_fds;
+        *n_storage_fds = rn_storage_fds;
 
         rfds = NULL;
         rfd_names = NULL;
 
-        return rn_fds;
+        return 0;
 }
 
 static bool service_exec_needs_notify_socket(Service *s, ExecFlags flags) {
@@ -1209,7 +1216,7 @@ static int service_spawn(
 
         _cleanup_strv_free_ char **final_env = NULL, **our_env = NULL, **fd_names = NULL;
         _cleanup_free_ int *fds = NULL;
-        unsigned n_fds = 0, n_socket_fds = 0, n_env = 0;
+        unsigned n_storage_fds = 0, n_socket_fds = 0, n_env = 0;
         const char *path;
         pid_t pid;
 
@@ -1253,12 +1260,11 @@ static int service_spawn(
             s->exec_context.std_output == EXEC_OUTPUT_SOCKET ||
             s->exec_context.std_error == EXEC_OUTPUT_SOCKET) {
 
-                r = service_collect_fds(s, &fds, &fd_names, &n_socket_fds);
+                r = service_collect_fds(s, &fds, &fd_names, &n_storage_fds, &n_socket_fds);
                 if (r < 0)
                         return r;
 
-                n_fds = r;
-                log_unit_debug(UNIT(s), "Passing %i fds to service", n_fds);
+                log_unit_debug(UNIT(s), "Passing %i fds to service", n_storage_fds + n_socket_fds);
         }
 
         r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), timeout));
@@ -1352,7 +1358,7 @@ static int service_spawn(
         exec_params.environment = final_env;
         exec_params.fds = fds;
         exec_params.fd_names = fd_names;
-        exec_params.n_fds = n_fds;
+        exec_params.n_storage_fds = n_storage_fds;
         exec_params.n_socket_fds = n_socket_fds;
         exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(s)->manager);
         exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported;