Re: [Libguestfs] [libnbd PATCH v3 08/29] lib/utils: add unit test for async-signal-safe assert()
On Wed, Feb 15, 2023 at 03:11:37PM +0100, Laszlo Ersek wrote:
Don't try to test async-signal-safety, only that
NBD_INTERNAL_FORK_SAFE_ASSERT() works similarly to assert():
- it prints diagnostics to stderr,
- it calls abort().
Some unfortunate gymnastics are necessary to avoid littering the system
with unwanted core dumps.
Wow - impressive work for a unit test.
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
configure.ac | 5 ++
lib/test-fork-safe-assert.c | 63 ++++++++++++++++++++
lib/Makefile.am | 38 ++++++++++--
lib/test-fork-safe-assert.sh | 31 ++++++++++
.gitignore | 2 +
5 files changed, 135 insertions(+), 4 deletions(-)
+++ b/lib/test-fork-safe-assert.c
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#ifdef HAVE_PRCTL
+#include <sys/prctl.h>
+#endif
+#include <sys/resource.h>
+
+#undef NDEBUG
+
+#include "internal.h"
+
+#define TRUE 1
+#define FALSE 0
Any reason for defining these, other than to test that our macro
properly stringifies them into the resulting assertion text?
/me reads ahead
Yep, your .sh script does check that a literal "`FALSE'" is part of
the expected output string. A comment here might be worthwhile?
+
+int
+main (void)
+{
+ struct rlimit rlimit;
+
+ /* The standard approach for disabling core dumping. Has no effect on Linux if
+ * /proc/sys/kernel/core_pattern starts with a pipe (|) symbol.
+ */
+ if (getrlimit (RLIMIT_CORE, &rlimit) == -1) {
+ perror ("getrlimit");
+ return EXIT_FAILURE;
+ }
+ rlimit.rlim_cur = 0;
+ if (setrlimit (RLIMIT_CORE, &rlimit) == -1) {
+ perror ("setrlimit");
+ return EXIT_FAILURE;
+ }
+
+#ifdef HAVE_PRCTL
+ if (prctl (PR_SET_DUMPABLE, 0, 0, 0, 0) == -1) {
+ perror ("prctl");
+ return EXIT_FAILURE;
Would 'return 77' (ie 'skipped test', in automake terminology) be
better if we can't pre-set the environment to our liking? [1]
+ }
+#endif
+
+ NBD_INTERNAL_FORK_SAFE_ASSERT (TRUE);
+ NBD_INTERNAL_FORK_SAFE_ASSERT (FALSE);
+ return 0;
+++ b/lib/test-fork-safe-assert.sh
@@ -0,0 +1,31 @@
+
+set +e
+
+./test-fork-safe-assert 2>test-fork-safe-assert.err
+exit_status=$?
+
+set -e
+
+test $exit_status -gt 128
[1] Hmm - if you do return 77 when we can't avoid the core dump, this
would need to forward that on.
+signal_name=$(kill -l $exit_status)
+test "x$signal_name" = xABRT || test "x$signal_name" = xSIGABRT
+
+ptrn="^test-fork-safe-assert\\.c:[0-9]+: main: Assertion \`FALSE'
failed\\.\$"
Would it be any easier writing ptrn with '' instead of "" shell
quoting?
Do we also want to test that the pattern "`TRUE'" is NOT present in
the output file (that is, our passing assertion does not generate
unexpected output)?
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