util: change the signal handler to a context-manager like macro

Change the signal-blocking helper to take a vararg list of signals to
block and provide a magic macro that works like python's context
manager, using  attribute(cleanup).

In our for loop we create a new struct with the old sigmask and a
boolean that's always true. We enter the body of the loop once and
set that boolean to false on the second run, i.e. we never do more than
one run. On loop exit, the destroy function for our struct restores the
previous signal mask.

src/util-io.c

@@ -47,9 +47,10 @@ xread_with_fds(int fd, void *buf, size_t count, int **fds)
 		.msg_controllen = sizeof(control),
 	};
 
-	sigset_t old_mask = signals_block();
-	int received = xerrno(recvmsg(fd, &msg, 0));
-	signals_release(old_mask);
+	int received = 0;
+	with_signals_blocked(SIGALRM)
+		received = xerrno(recvmsg(fd, &msg, 0));
+
 	if (received > 0) {
 		*fds = NULL;
 
@@ -112,11 +113,10 @@ xsend_with_fd(int fd, const void *buf, size_t len, int *fds)
 	header->cmsg_type = SCM_RIGHTS;
 	memcpy(CMSG_DATA(CMSG_FIRSTHDR(&msg)), fds, nfds * sizeof(int));
 
-	sigset_t old_mask = signals_block();
-	int sent = xerrno(sendmsg(fd, &msg, MSG_NOSIGNAL));
-	signals_release(old_mask);
+	with_signals_blocked(SIGALRM)
+		return xerrno(sendmsg(fd, &msg, MSG_NOSIGNAL));
 
-	return sent;
+	abort(); // can't be reached
 }
 
 /* consider this struct opaque */
@@ -558,4 +558,47 @@ MUNIT_TEST(test_pass_fd)
 
 	return MUNIT_OK;
 }
+
+static inline void
+sigblock_helper(void) {
+	with_signals_blocked(SIGPIPE, SIGALRM) {
+		break; /* breaking out of loop must clean up too */
+	}
+}
+
+MUNIT_TEST(test_signal_blocker)
+{
+	int rc;
+	sigset_t mask;
+	int count = 0;
+
+	with_signals_blocked(SIGPIPE, SIGALRM) {
+		rc = sigprocmask(SIG_BLOCK, NULL, &mask);
+		munit_assert_int(rc, !=, -1);
+
+		munit_assert(sigismember(&mask, SIGPIPE));
+		munit_assert(sigismember(&mask, SIGALRM));
+		munit_assert(!sigismember(&mask, SIGINT)); /* We didn't touch that one */
+
+		++count;
+	}
+	munit_assert_int(count, ==, 1); /* loop body only entered once */
+
+	rc = sigprocmask(SIG_BLOCK, NULL, &mask);
+	munit_assert_int(rc, !=, -1);
+
+	munit_assert(!sigismember(&mask, SIGPIPE));
+	munit_assert(!sigismember(&mask, SIGALRM));
+
+	sigblock_helper();
+
+	rc = sigprocmask(SIG_BLOCK, NULL, &mask);
+	munit_assert_int(rc, !=, -1);
+
+	munit_assert(!sigismember(&mask, SIGPIPE));
+	munit_assert(!sigismember(&mask, SIGALRM));
+
+	return MUNIT_OK;
+}
+
 #endif

src/util-io.h

@@ -39,34 +39,70 @@
 #include "util-strings.h"
 #include "util-mem.h"
 
-
 /**
- * Blocks the SIGALRM signal.
+ * Blocks the zero-terminated list of signals
  *
  * @return the previous set of signals to pass to signals_release()
  */
 static inline sigset_t
-signals_block(void)
+signals_block(int signal, ...)
 {
 	sigset_t old_mask;
 	sigset_t new_mask;
+	va_list sigs;
+	va_start(sigs, signal);
+	int sigcount = 0;
 
 	sigemptyset(&new_mask);
-	sigaddset(&new_mask, SIGALRM);
+
+	while (signal != 0) {
+		sigaddset(&new_mask, signal);
+		signal = va_arg(sigs, int);
+
+		assert(++sigcount < 16); /* likely missing zero-terminator */
+	}
 	sigprocmask(SIG_BLOCK, &new_mask, &old_mask);
 
 	return old_mask;
 }
 
 /**
- * Release the SIGALRM signal.
+ * Release signals and revert back to the old sigmask
  *
  * @param mask: The previous set of signals as returned by signals_block().
+ * @return Always 0
  */
-static inline void
+static inline int
 signals_release(sigset_t mask)
 {
 	sigprocmask(SIG_SETMASK, &mask, NULL);
+	return 0;
+}
+
+/**
+ * Python-like context manager for blocking signals, e.g.
+ *
+ * void foo(void) {
+ *     ...
+ *     with_signals_blocked(SIGALARM, SIGINT) {
+ *          do_something();
+ *     }
+ *     // signals are unblocked again.
+ */
+#define with_signals_blocked(...) \
+    for (_cleanup_(signal_blocked_destroy) struct _sigblock b_ = { .mask = signals_block(__VA_ARGS__, 0), .is_blocked = true };  \
+	 b_.is_blocked; \
+	 b_.is_blocked = false)
+
+struct _sigblock {
+	sigset_t mask;
+	bool is_blocked;
+};
+
+static inline void
+signal_blocked_destroy(struct _sigblock *s)
+{
+	signals_release(s->mask);
 }
 
 /**
@@ -86,9 +122,10 @@ xerrno(int value) {
  */
 static inline int
 xclose(int fd) {
-	sigset_t old_mask = signals_block();
-	if (fd != -1) close(fd);
-	signals_release(old_mask);
+	if (fd != -1) {
+		with_signals_blocked(SIGALRM)
+			close(fd);
+	}
 
 	return -1;
 }
@@ -106,11 +143,10 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(FILE *, fclose);
 static inline int
 xread(int fd, void *buf, size_t count)
 {
-	sigset_t old_mask = signals_block();
-	int n = xerrno(read(fd, buf, count));
-	signals_release(old_mask);
+	with_signals_blocked(SIGALRM)
+		return xerrno(read(fd, buf, count));
 
-	return n;
+	abort(); // can't be reached
 }
 
 /**
@@ -130,11 +166,9 @@ xread_with_fds(int fd, void *buf, size_t count, int **fds);
 static inline int
 xwrite(int fd, const void *buf, size_t count)
 {
-	sigset_t old_mask = signals_block();
-	int n = xerrno(write(fd, buf, count));
-	signals_release(old_mask);
-
-	return n;
+	with_signals_blocked(SIGALRM)
+		return xerrno(write(fd, buf, count));
+	abort(); // can't be reached
 }
 
 /**
@@ -145,11 +179,10 @@ xwrite(int fd, const void *buf, size_t count)
 static inline int
 xsend(int fd, const void *buf, size_t len)
 {
-	sigset_t old_mask = signals_block();
-	int n = xerrno(send(fd, buf, len, MSG_NOSIGNAL));
-	signals_release(old_mask);
+	with_signals_blocked(SIGALRM)
+		return xerrno(send(fd, buf, len, MSG_NOSIGNAL));
 
-	return n;
+	abort(); // can't be reached
 }
 
 /**
@@ -158,11 +191,10 @@ xsend(int fd, const void *buf, size_t len)
 static inline int
 xpipe2(int pipefd[2], int flags)
 {
-	sigset_t old_mask = signals_block();
-	int n = pipe2(pipefd, flags);
-	signals_release(old_mask);
+	with_signals_blocked(SIGALRM)
+		return pipe2(pipefd, flags);
 
-	return n;
+	abort(); // can't be reached
 }
 
 /**
@@ -171,11 +203,10 @@ xpipe2(int pipefd[2], int flags)
 static inline int
 xdup(int fd)
 {
-	sigset_t old_mask = signals_block();
-	int n = dup(fd);
-	signals_release(old_mask);
+	with_signals_blocked(SIGALRM)
+		return dup(fd);
 
-	return n;
+	abort(); // can't be reached
 }
 
 /**
@@ -201,19 +232,17 @@ xconnect(const char *path)
 	if (!xsnprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path))
 		return -EINVAL;
 
-	sigset_t old_mask = signals_block();
-	int sockfd = socket(AF_UNIX, SOCK_STREAM|SOCK_NONBLOCK, 0);
-	if (sockfd == -1)
-		goto fail;
+	int sockfd = -1;
 
-	if (connect(sockfd, (struct sockaddr*)&addr, sizeof(addr)) == -1)
-		goto fail;
+	with_signals_blocked(SIGALRM) {
+		sockfd = socket(AF_UNIX, SOCK_STREAM|SOCK_NONBLOCK, 0);
+		if (sockfd == -1)
+			break;
 
-	signals_release(old_mask);
-	return sockfd;
-fail:
-	signals_release(old_mask);
-	return -errno;
+		if (connect(sockfd, (struct sockaddr*)&addr, sizeof(addr)) == -1)
+			break;
+	}
+	return sockfd == -1 ? -errno : sockfd;
 }
 
 /**