[LTP] [PATCH] [RFC] lib/tst_test.c: Fix tst_brk() handling

[Cyril Hrubis <chrubis@suse.cz>]

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


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp