SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
'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.
This commit is contained in:
Franck Bui
2017-06-08 16:21:35 +02:00
parent 9b1419111a
commit 4c47affcf1
3 changed files with 49 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/execute.c
+19 -11
View File
@@ -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] &&
src/core/execute.h
+1 -1
View File
@@ -246,7 +246,7 @@ struct ExecParameters {
int *fds;
char **fd_names;
unsigned n_fds;
unsigned n_storage_fds;
unsigned n_socket_fds;
ExecFlags flags;
src/core/service.c
+29 -23
View File
@@ -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;