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 <ler...@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 <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
