[LTP] [PATCH] [RFC] lib/tst_test.c: Fix tst_brk() handling
Cyril Hrubis
chrubis@suse.cz
Fri Nov 15 17:41:01 CET 2024
This makes the tst_brk() handling cleaner and saner as instead of
propagating the tst_brk() result in a return value an abort flag is
introduced into the shared memory.
Now:
- All the processes but the library one that reports the results exit
with 0
- tst_brk(TBROK, ...) increments result conters, sets the abort flag.
and exit current process
- all other tst_brk() variants will just increments the countes and
exits the current process
This makes the tst_brk() behavior well defined so we can now even call
tst_brk() with TFAIL and TPASS as well.
Open question (may be done in a separatep patch):
Should tst_brk(TBROK, ...) apart from setting the flag also send sigkill
signal to the test process group to kill any leftover test processes?
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
include/tst_test.h | 24 +++++++-------
lib/newlib_tests/.gitignore | 6 +++-
lib/newlib_tests/test23.c | 25 ++++++++++++++
lib/newlib_tests/test24.c | 25 ++++++++++++++
lib/newlib_tests/test25.c | 19 +++++++++++
lib/newlib_tests/test26.c | 20 +++++++++++
lib/tst_test.c | 66 +++++++++++++++++++++----------------
7 files changed, 144 insertions(+), 41 deletions(-)
create mode 100644 lib/newlib_tests/test23.c
create mode 100644 lib/newlib_tests/test24.c
create mode 100644 lib/newlib_tests/test25.c
create mode 100644 lib/newlib_tests/test26.c
diff --git a/include/tst_test.h b/include/tst_test.h
index 8d1819f74..7ad91e9fa 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -98,28 +98,30 @@ void tst_brk_(const char *file, const int lineno, int ttype,
__attribute__ ((format (printf, 4, 5)));
/**
- * tst_brk() - Reports a breakage and exits the test.
+ * tst_brk() - Reports a breakage and exits the test or test process.
*
* @ttype: An enum tst_res_type.
* @arg_fmt: A printf-like format.
* @...: A printf-like parameters.
*
- * Reports either TBROK or TCONF and exits the test immediately. When called
- * all children in the same process group as the main test library process are
- * killed. This function, unless in a test cleanup, calls _exit() and does not
- * return.
+ * Reports a single result and exits immediately. The call behaves differently
+ * based on the ttype parameter. For all ttype results but TBROK the call exits
+ * the current test process, i.e. increments test result counters and calls
+ * exit(0).
+ *
+ * The TBROK ttype is special that apart from exitting the current test process
+ * it also tells to the test library to exit immediatelly. This means that any
+ * subsequent test iterations are not executed, e.g. if tests runs for all
+ * filesystems but tst_brk() with TBROK is called, the test exits and does not
+ * attempt to continue a test iteration for the next filtesystem.
*
* When test is in cleanup() function TBROK is converted into TWARN by the test
* library and we attempt to carry on with a cleanup even when tst_brk() was
* called. This makes it possible to use SAFE_FOO() macros in the test cleanup
* without interrupting the cleanup process on a failure.
*/
-#define tst_brk(ttype, arg_fmt, ...) \
- ({ \
- TST_BRK_SUPPORTS_ONLY_TCONF_TBROK(!((ttype) & \
- (TBROK | TCONF | TFAIL))); \
- tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\
- })
+#define tst_brk(ttype, arg_fmt, ...) \
+ tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__)
void tst_printf(const char *const fmt, ...)
__attribute__((nonnull(1), format (printf, 1, 2)));
diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index 6d125f933..b3b662d51 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -23,6 +23,10 @@ tst_print_result
test19
test20
test22
+test23
+test24
+test25
+test26
tst_expiration_timer
test_assert
test_timer
@@ -58,4 +62,4 @@ test_runtime01
test_runtime02
test_children_cleanup
tst_res_flags
-tst_safe_sscanf
\ No newline at end of file
+tst_safe_sscanf
diff --git a/lib/newlib_tests/test23.c b/lib/newlib_tests/test23.c
new file mode 100644
index 000000000..f3ec29793
--- /dev/null
+++ b/lib/newlib_tests/test23.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Test that tst_brk(TFAIL, ...) works properly.
+ */
+#include "tst_test.h"
+
+static void do_test(void)
+{
+ int pid = SAFE_FORK();
+
+ if (pid)
+ return;
+
+ tst_brk(TFAIL, "Test child exitting...");
+ tst_res(TWARN, "Test child stil alive!");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+ .forks_child = 1,
+};
diff --git a/lib/newlib_tests/test24.c b/lib/newlib_tests/test24.c
new file mode 100644
index 000000000..423383cfd
--- /dev/null
+++ b/lib/newlib_tests/test24.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Test that tst_brk(TPASS, ...) works properly.
+ */
+#include "tst_test.h"
+
+static void do_test(void)
+{
+ int pid = SAFE_FORK();
+
+ if (pid)
+ return;
+
+ tst_brk(TPASS, "Test child exitting...");
+ tst_res(TWARN, "Test child stil alive!");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+ .forks_child = 1,
+};
diff --git a/lib/newlib_tests/test25.c b/lib/newlib_tests/test25.c
new file mode 100644
index 000000000..0795ca7d0
--- /dev/null
+++ b/lib/newlib_tests/test25.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Test that tst_brk(TBROK, ...) exits the test immediatelly.
+ */
+#include "tst_test.h"
+
+static void do_test(void)
+{
+ tst_brk(TBROK, "Exitting the test during the first variant");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+ .test_variants = 10,
+};
diff --git a/lib/newlib_tests/test26.c b/lib/newlib_tests/test26.c
new file mode 100644
index 000000000..27829c703
--- /dev/null
+++ b/lib/newlib_tests/test26.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Test that tst_brk(TFAIL, ...) exits only single test variant.
+ */
+#include "tst_test.h"
+
+static void do_test(void)
+{
+ tst_brk(TFAIL, "Failing a test variant");
+ tst_res(TWARN, "Shouldn't be reached");
+}
+
+static struct tst_test test = {
+ .test_all = do_test,
+ .test_variants = 10,
+};
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 8db554dea..9dd00718a 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -71,6 +71,13 @@ struct results {
int failed;
int warnings;
int broken;
+
+ /*
+ * This is set by a call to tst_brk() with TBROK parameter and means
+ * that the test should exit immediatelly.
+ */
+ int abort_flag;
+
unsigned int timeout;
int max_runtime;
};
@@ -395,7 +402,16 @@ void tst_vbrk_(const char *file, const int lineno, int ttype, const char *fmt,
if (getpid() == lib_pid)
do_exit(TTYPE_RESULT(ttype));
- exit(TTYPE_RESULT(ttype));
+ /*
+ * If we get here we are in a child process, either the main child
+ * running the test or its children. If any of them called tst_brk()
+ * with TBROK we need to exit the test. Otherwise we just exit the
+ * current process.
+ */
+ if (TTYPE_RESULT(ttype) == TBROK)
+ tst_atomic_inc(&results->abort_flag);
+
+ exit(0);
}
void tst_res_(const char *file, const int lineno, int ttype,
@@ -432,8 +448,6 @@ void tst_printf(const char *const fmt, ...)
static void check_child_status(pid_t pid, int status)
{
- int ret;
-
if (WIFSIGNALED(status)) {
tst_brk(TBROK, "Child (%i) killed by signal %s", pid,
tst_strsig(WTERMSIG(status)));
@@ -442,15 +456,8 @@ static void check_child_status(pid_t pid, int status)
if (!(WIFEXITED(status)))
tst_brk(TBROK, "Child (%i) exited abnormally", pid);
- ret = WEXITSTATUS(status);
- switch (ret) {
- case TPASS:
- case TBROK:
- case TCONF:
- break;
- default:
- tst_brk(TBROK, "Invalid child (%i) exit value %i", pid, ret);
- }
+ if (WEXITSTATUS(status))
+ tst_brk(TBROK, "Invalid child (%i) exit value %i", pid, WEXITSTATUS(status));
}
void tst_reap_children(void)
@@ -915,6 +922,14 @@ static void print_failure_hints(void)
show_failure_hints = 0;
}
+/*
+ * Prints results, cleans up after the test library and exits the test library
+ * process. The ret parameter is used to pass the result flags in a case of a
+ * failure before we managed to set up the shared memory where we store the
+ * results. This allows us to use SAFE_MACROS() in the initialization of the
+ * shared memory. The ret parameter is not used (passed 0) when called
+ * explicitly from the rest of the library.
+ */
static void do_exit(int ret)
{
if (results) {
@@ -1555,6 +1570,7 @@ static void run_tests(void)
if (results_equal(&saved_results, results))
tst_brk(TBROK, "Test haven't reported results!");
+
return;
}
@@ -1775,7 +1791,7 @@ static int fork_testrun(void)
tst_res(TINFO, "Killed the leftover descendant processes");
if (WIFEXITED(status) && WEXITSTATUS(status))
- return WEXITSTATUS(status);
+ tst_brk(TBROK, "Child returned with %i", WEXITSTATUS(status));
if (WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL) {
tst_res(TINFO, "If you are running on slow machine, "
@@ -1856,15 +1872,10 @@ static int run_tcases_per_fs(void)
if (!fs)
continue;
- ret = run_tcase_on_fs(fs, filesystems[i]);
+ run_tcase_on_fs(fs, filesystems[i]);
- if (ret == TCONF)
- continue;
-
- if (ret == 0)
- continue;
-
- do_exit(ret);
+ if (tst_atomic_load(&results->abort_flag))
+ do_exit(0);
}
return ret;
@@ -1874,7 +1885,6 @@ unsigned int tst_variant;
void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
{
- int ret = 0;
unsigned int test_variants = 1;
struct utsname uval;
@@ -1903,19 +1913,17 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
if (tst_test->all_filesystems || count_fs_descs() > 1)
- ret |= run_tcases_per_fs();
+ run_tcases_per_fs();
else
- ret |= fork_testrun();
+ fork_testrun();
- if (ret & ~(TCONF))
- goto exit;
+ if (tst_atomic_load(&results->abort_flag))
+ do_exit(0);
}
-exit:
- do_exit(ret);
+ do_exit(0);
}
-
void tst_flush(void)
{
int rval;
--
2.45.2
More information about the ltp
mailing list