2:57 p.m.
On Wed, Feb 15, 2023 at 03:11:36PM +0100, Laszlo Ersek wrote:
Add an assert() variant that we may call between fork() and exec*().
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
+++ b/lib/internal.h
+
+#ifdef NDEBUG
+#define NBD_INTERNAL_FORK_SAFE_ASSERT(expression) ((void)0)
May be affected by outcome of side discussion on 01/29 about whether
we want space after cast.
+#else
+#define NBD_INTERNAL_FORK_SAFE_ASSERT(expression) \
+ (nbd_internal_fork_safe_assert ((expression) != 0, __FILE__, __LINE__, \
+ __func__, #expression))
+#endif
+
#endif /* LIBNBD_INTERNAL_H */
+++ b/lib/utils.c
@@ -431,13 +431,35 @@ nbd_internal_socketpair (int domain, int type, int protocol, int
*fds)
if (ret == 0) {
for (i = 0; i < 2; i++) {
if (fcntl (fds[i], F_SETFD, FD_CLOEXEC) == -1) {
close (fds[0]);
close (fds[1]);
return -1;
}
}
}
#endif
return ret;
}
+
+void
+nbd_internal_fork_safe_assert (int result, const char *file, long line,
+ const char *func, const char *assertion)
+{
+ const char *line_out;
+ char line_buf[32];
+
+ if (result)
+ return;
Since no code should ever directly call
nbd_internal_fork_safe_assert(0,...), but instead go through our macro
that has already filtered on expression's value, this conditional is
technically dead code; but I'm okay whether it stays in or gets
removed.
+
+ xwrite (STDERR_FILENO, file, strlen (file));
+ xwrite (STDERR_FILENO, ":", 1);
Presumably, if our first best-effort xwrite() fails to produce full
output, all later xwrite() will hopefully hit the same error condition
so that we aren't producing even more-mangled output where later
syscalls succeed despite missing context earlier in the overall
output. If it were something we truly wanted to worry about, the
solution would be pre-loading the entire message into a single buffer,
then calling xwrite() just once - but that's far more effort for
something we don't anticipate hitting in normal usage anyways. I'm
happy if you ignore this whole paragraph of mine.
+ line_out = nbd_internal_fork_safe_itoa (line, line_buf, sizeof
line_buf);
+ xwrite (STDERR_FILENO, line_out, strlen (line_out));
+ xwrite (STDERR_FILENO, ": ", 2);
+ xwrite (STDERR_FILENO, func, strlen (func));
+ xwrite (STDERR_FILENO, ": Assertion `", 13);
+ xwrite (STDERR_FILENO, assertion, strlen (assertion));
+ xwrite (STDERR_FILENO, "' failed.\n", 10);
These days, the quoting style `' seems to be disappearing in favor of
''. But a quick test showed me that at least glibc 2.36 is still
producing the message with the quotes as you have spelled it here.
+ abort ();
Huh. I checked the source to glibc's assert.c, where it includes a
comment about having to work cleanly even if a second assertion ends
up being raised from within a SIGABRT handler active during the first
attempt to abort(). But POSIX says that abort() shall attempt to
override any SIGABRT handler in place, so I wonder if glibc's
implementation is a bit too defensive. At any rate, your
implementation looks reasonable, and more to the point, satisfies your
desire of being async-signal-safe and thus appropriate between fork()
and exec().
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org