[bpf-next,2/5] selftests/bpf: Replace CHECK with ASSERT_* in ns_current_pid_tgid test

Commit Message

Yonghong Song March 7, 2024, 11:27 p.m. UTC

Replace CHECK in selftest ns_current_pid_tgid with recommended ASSERT_* style.
I also shortened subtest name as the prefix of subtest name is covered
by the test name already.

This patch does fix a testing issue. Currently even if bss->user_{pid,tgid}
is not correct, the test still passed since the clone func returns 0.
I fixed it to return a non-zero value if bss->user_{pid,tgid} is incorrect.

Signed-off-by: Yonghong Song <[e-mail]>
---
 .../bpf/prog_tests/ns_current_pid_tgid.c      | 36 ++++++++++---------
 1 file changed, 19 insertions(+), 17 deletions(-)

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
index 24d493482ffc..3a0664a86243 100644
--- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
@@ -20,19 +20,19 @@  static int test_current_pid_tgid(void *args)
 {
 	struct test_ns_current_pid_tgid__bss  *bss;
 	struct test_ns_current_pid_tgid *skel;
-	int err = -1, duration = 0;
+	int ret = -1, err;
 	pid_t tgid, pid;
 	struct stat st;
 
 	skel = test_ns_current_pid_tgid__open_and_load();
-	if (CHECK(!skel, "skel_open_load", "failed to load skeleton\n"))
-		goto cleanup;
+	if (!ASSERT_OK_PTR(skel, "test_ns_current_pid_tgid__open_and_load"))
+		goto out;
 
 	pid = syscall(SYS_gettid);
 	tgid = getpid();
 
 	err = stat("/proc/self/ns/pid", &st);
-	if (CHECK(err, "stat", "failed /proc/self/ns/pid: %d\n", err))
+	if (!ASSERT_OK(err, "stat /proc/self/ns/pid"))
 		goto cleanup;
 
 	bss = skel->bss;
@@ -42,24 +42,26 @@  static int test_current_pid_tgid(void *args)
 	bss->user_tgid = 0;
 
 	err = test_ns_current_pid_tgid__attach(skel);
-	if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
+	if (!ASSERT_OK(err, "test_ns_current_pid_tgid__attach"))
 		goto cleanup;
 
 	/* trigger tracepoint */
 	usleep(1);
-	ASSERT_EQ(bss->user_pid, pid, "pid");
-	ASSERT_EQ(bss->user_tgid, tgid, "tgid");
-	err = 0;
+	if (!ASSERT_EQ(bss->user_pid, pid, "pid"))
+		goto cleanup;
+	if (!ASSERT_EQ(bss->user_tgid, tgid, "tgid"))
+		goto cleanup;
+	ret = 0;
 
 cleanup:
-	 test_ns_current_pid_tgid__destroy(skel);
-
-	return err;
+	test_ns_current_pid_tgid__destroy(skel);
+out:
+	return ret;
 }
 
 static void test_ns_current_pid_tgid_new_ns(void)
 {
-	int wstatus, duration = 0;
+	int wstatus;
 	pid_t cpid;
 
 	/* Create a process in a new namespace, this process
@@ -68,21 +70,21 @@  static void test_ns_current_pid_tgid_new_ns(void)
 	cpid = clone(test_current_pid_tgid, child_stack + STACK_SIZE,
 		     CLONE_NEWPID | SIGCHLD, NULL);
 
-	if (CHECK(cpid == -1, "clone", "%s\n", strerror(errno)))
+	if (!ASSERT_NEQ(cpid, -1, "clone"))
 		return;
 
-	if (CHECK(waitpid(cpid, &wstatus, 0) == -1, "waitpid", "%s\n", strerror(errno)))
+	if (!ASSERT_NEQ(waitpid(cpid, &wstatus, 0), -1, "waitpid"))
 		return;
 
-	if (CHECK(WEXITSTATUS(wstatus) != 0, "newns_pidtgid", "failed"))
+	if (!ASSERT_OK(WEXITSTATUS(wstatus), "newns_pidtgid"))
 		return;
 }
 
 /* TODO: use a different tracepoint */
 void serial_test_ns_current_pid_tgid(void)
 {
-	if (test__start_subtest("ns_current_pid_tgid_root_ns"))
+	if (test__start_subtest("root_ns_tp"))
 		test_current_pid_tgid(NULL);
-	if (test__start_subtest("ns_current_pid_tgid_new_ns"))
+	if (test__start_subtest("new_ns_tp"))
 		test_ns_current_pid_tgid_new_ns();
 }