From: Franck Bui Date: Thu, 8 Jun 2017 13:41:26 +0000 (+0200) Subject: core: remove the redundancy of 'n_fds' and 'n_storage_fds' in ExecParameters struct X-Git-Tag: v234~101^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4c47affcf1e501431f3cb567c516ec252dfc4bbc;p=thirdparty%2Fsystemd.git 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 7d9fc037b92..550a8cfd396 100644 --- 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 dee7af14239..136319359c0 100644 --- 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 370a6316e7f..df7f1f3053a 100644 --- 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;