diff options
| -rw-r--r-- | src/nspawn/nspawn.c | 4 | ||||
| -rw-r--r-- | src/shared/barrier.c | 114 | ||||
| -rw-r--r-- | src/shared/barrier.h | 4 | ||||
| -rw-r--r-- | src/shared/pty.c | 3 | ||||
| -rw-r--r-- | src/test/test-barrier.c | 4 |
5 files changed, 50 insertions, 79 deletions
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 657512d66..7c47f6ecd 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3073,13 +3073,13 @@ int main(int argc, char *argv[]) { for (;;) { ContainerStatus container_status; - _cleanup_(barrier_destroy) Barrier barrier = { }; + _cleanup_(barrier_destroy) Barrier barrier = BARRIER_NULL; struct sigaction sa = { .sa_handler = nop_handler, .sa_flags = SA_NOCLDSTOP, }; - r = barrier_init(&barrier); + r = barrier_create(&barrier); if (r < 0) { log_error("Cannot initialize IPC barrier: %s", strerror(-r)); goto finish; diff --git a/src/shared/barrier.c b/src/shared/barrier.c index c198329cb..4ac42d023 100644 --- a/src/shared/barrier.c +++ b/src/shared/barrier.c @@ -93,7 +93,7 @@ */ /** - * barrier_init() - Initialize a barrier object + * barrier_create() - Initialize a barrier object * @obj: barrier to initialize * * This initializes a barrier object. The caller is responsible of allocating @@ -111,26 +111,16 @@ * * Returns: 0 on success, negative error code on failure. */ -int barrier_init(Barrier *obj) { - _cleanup_(barrier_destroy) Barrier b = { }; - int r; - - assert_return(obj, -EINVAL); - - b.me = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); - if (b.me < 0) - return -errno; - - b.them = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); - if (b.them < 0) - return -errno; +int barrier_create(Barrier *b) { + assert(b); - r = pipe2(b.pipe, O_CLOEXEC | O_NONBLOCK); - if (r < 0) + if ((b->me = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)) < 0 || + (b->them = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)) < 0 || + pipe2(b->pipe, O_CLOEXEC | O_NONBLOCK) < 0) { + barrier_destroy(b); return -errno; + } - memcpy(obj, &b, sizeof(b)); - zero(b); return 0; } @@ -138,15 +128,11 @@ int barrier_init(Barrier *obj) { * barrier_destroy() - Destroy a barrier object * @b: barrier to destroy or NULL * - * This destroys a barrier object that has previously been initialized via - * barrier_init(). The object is released and reset to invalid state. - * Therefore, it is safe to call barrier_destroy() multiple times or even if - * barrier_init() failed. However, you must not call barrier_destroy() if you - * never called barrier_init() on the object before. - * - * It is safe to initialize a barrier via zero() / memset(.., 0, ...). Even - * though it has embedded FDs, barrier_destroy() can deal with zeroed objects - * just fine. + * This destroys a barrier object that has previously been passed to + * barrier_create(). The object is released and reset to invalid + * state. Therefore, it is safe to call barrier_destroy() multiple + * times or even if barrier_create() failed. However, barrier must be + * always initalized with BARRIER_NULL. * * If @b is NULL, this is a no-op. */ @@ -154,21 +140,9 @@ void barrier_destroy(Barrier *b) { if (!b) return; - /* @me and @them cannot be both FD 0. Lets be pedantic and check the - * pipes and barriers, too. If all are 0, the object was zero()ed and - * is invalid. This allows users to use zero(barrier) to reset the - * backing memory. */ - if (b->me == 0 && - b->them == 0 && - b->pipe[0] == 0 && - b->pipe[1] == 0 && - b->barriers == 0) - return; - b->me = safe_close(b->me); b->them = safe_close(b->them); - b->pipe[0] = safe_close(b->pipe[0]); - b->pipe[1] = safe_close(b->pipe[1]); + safe_close_pair(b->pipe); b->barriers = 0; } @@ -177,13 +151,14 @@ void barrier_destroy(Barrier *b) { * @b: barrier to operate on * @role: role to set on the barrier * - * This sets the roles on a barrier object. This is needed to know which - * side of the barrier you're on. Usually, the parent creates the barrier via - * barrier_init() and then calls fork() or clone(). Therefore, the FDs are - * duplicated and the child retains the same barrier object. + * This sets the roles on a barrier object. This is needed to know + * which side of the barrier you're on. Usually, the parent creates + * the barrier via barrier_create() and then calls fork() or clone(). + * Therefore, the FDs are duplicated and the child retains the same + * barrier object. * - * Both sides need to call barrier_set_role() after fork() or clone() are done. - * If this is not done, barriers will not work correctly. + * Both sides need to call barrier_set_role() after fork() or clone() + * are done. If this is not done, barriers will not work correctly. * * Note that barriers could be supported without fork() or clone(). However, * this is currently not needed so it hasn't been implemented. @@ -196,9 +171,9 @@ void barrier_set_role(Barrier *b, unsigned int role) { /* make sure this is only called once */ assert(b->pipe[1] >= 0 && b->pipe[1] >= 0); - if (role == BARRIER_PARENT) { + if (role == BARRIER_PARENT) b->pipe[1] = safe_close(b->pipe[1]); - } else { + else { b->pipe[0] = safe_close(b->pipe[0]); /* swap me/them for children */ @@ -218,7 +193,7 @@ static bool barrier_write(Barrier *b, uint64_t buf) { do { len = write(b->me, &buf, sizeof(buf)); - } while (len < 0 && (errno == EAGAIN || errno == EINTR)); + } while (len < 0 && IN_SET(errno, EAGAIN, EINTR)); if (len != sizeof(buf)) goto error; @@ -229,9 +204,8 @@ static bool barrier_write(Barrier *b, uint64_t buf) { b->barriers = BARRIER_WE_ABORTED; else b->barriers = BARRIER_I_ABORTED; - } else if (!barrier_is_aborted(b)) { + } else if (!barrier_is_aborted(b)) b->barriers += buf; - } return !barrier_i_aborted(b); @@ -241,52 +215,48 @@ error: * pipe-ends and treat this as abortion. The other end will notice the * pipe-close and treat it as abortion, too. */ - b->pipe[0] = safe_close(b->pipe[0]); - b->pipe[1] = safe_close(b->pipe[1]); + safe_close_pair(b->pipe); b->barriers = BARRIER_WE_ABORTED; return false; } /* waits for barriers; returns false if they aborted, otherwise true */ static bool barrier_read(Barrier *b, int64_t comp) { - uint64_t buf; - ssize_t len; - struct pollfd pfd[2] = { }; - int r; - if (barrier_they_aborted(b)) return false; while (b->barriers > comp) { - pfd[0].fd = (b->pipe[0] >= 0) ? b->pipe[0] : b->pipe[1]; - pfd[0].events = POLLHUP; - pfd[0].revents = 0; - pfd[1].fd = b->them; - pfd[1].events = POLLIN; - pfd[1].revents = 0; + struct pollfd pfd[2] = { + { .fd = b->pipe[0] >= 0 ? b->pipe[0] : b->pipe[1], + .events = POLLHUP }, + { .fd = b->them, + .events = POLLIN }}; + uint64_t buf; + int r; r = poll(pfd, 2, -1); - if (r < 0 && (errno == EAGAIN || errno == EINTR)) + if (r < 0 && IN_SET(errno, EAGAIN, EINTR)) continue; else if (r < 0) goto error; if (pfd[1].revents) { - /* events on @them signal us new data */ + ssize_t len; + + /* events on @them signal new data for us */ len = read(b->them, &buf, sizeof(buf)); - if (len < 0 && (errno == EAGAIN || errno == EINTR)) + if (len < 0 && IN_SET(errno, EAGAIN, EINTR)) continue; if (len != sizeof(buf)) goto error; - } else if (pfd[0].revents & (POLLHUP | POLLERR | POLLNVAL)) { + } else if (pfd[0].revents & (POLLHUP | POLLERR | POLLNVAL)) /* POLLHUP on the pipe tells us the other side exited. * We treat this as implicit abortion. But we only * handle it if there's no event on the eventfd. This * guarantees that exit-abortions do not overwrite real * barriers. */ buf = BARRIER_ABORTION; - } /* lock if they aborted */ if (buf >= (uint64_t)BARRIER_ABORTION) { @@ -294,9 +264,8 @@ static bool barrier_read(Barrier *b, int64_t comp) { b->barriers = BARRIER_WE_ABORTED; else b->barriers = BARRIER_THEY_ABORTED; - } else if (!barrier_is_aborted(b)) { + } else if (!barrier_is_aborted(b)) b->barriers -= buf; - } } return !barrier_they_aborted(b); @@ -307,8 +276,7 @@ error: * pipe-ends and treat this as abortion. The other end will notice the * pipe-close and treat it as abortion, too. */ - b->pipe[0] = safe_close(b->pipe[0]); - b->pipe[1] = safe_close(b->pipe[1]); + safe_close_pair(b->pipe); b->barriers = BARRIER_WE_ABORTED; return false; } diff --git a/src/shared/barrier.h b/src/shared/barrier.h index 7f76ec791..53b4439fb 100644 --- a/src/shared/barrier.h +++ b/src/shared/barrier.h @@ -57,7 +57,9 @@ struct Barrier { int64_t barriers; }; -int barrier_init(Barrier *obj); +#define BARRIER_NULL {-1, -1, {-1, -1}, 0} + +int barrier_create(Barrier *obj); void barrier_destroy(Barrier *b); void barrier_set_role(Barrier *b, unsigned int role); diff --git a/src/shared/pty.c b/src/shared/pty.c index 11d76f825..2863da489 100644 --- a/src/shared/pty.c +++ b/src/shared/pty.c @@ -105,6 +105,7 @@ int pty_new(Pty **out) { pty->ref = 1; pty->fd = -1; + pty->barrier = (Barrier) BARRIER_NULL; pty->fd = posix_openpt(O_RDWR | O_NOCTTY | O_CLOEXEC | O_NONBLOCK); if (pty->fd < 0) @@ -127,7 +128,7 @@ int pty_new(Pty **out) { if (r < 0) return -errno; - r = barrier_init(&pty->barrier); + r = barrier_create(&pty->barrier); if (r < 0) return r; diff --git a/src/test/test-barrier.c b/src/test/test-barrier.c index 640e50867..672a51e2c 100644 --- a/src/test/test-barrier.c +++ b/src/test/test-barrier.c @@ -59,10 +59,10 @@ static void msleep(unsigned long msecs) { #define TEST_BARRIER(_FUNCTION, _CHILD_CODE, _WAIT_CHILD, _PARENT_CODE, _WAIT_PARENT) \ static void _FUNCTION(void) { \ - Barrier b; \ + Barrier b = BARRIER_NULL; \ pid_t pid1, pid2; \ \ - assert_se(barrier_init(&b) >= 0); \ + assert_se(barrier_create(&b) >= 0); \ \ pid1 = fork(); \ assert_se(pid1 >= 0); \ |