Re: [PATCH bpf-next v2 1/5] selftests/bpf: Replace CHECK with ASSERT macros for ksyms test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3/22/24 9:13 AM, Andrii Nakryiko wrote:
On Thu, Mar 21, 2024 at 1:01 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
I am going to modify ksyms test later so take this opportunity
to replace old CHECK macros with new ASSERT macros.
No functionality change.

Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
---
  .../testing/selftests/bpf/prog_tests/ksyms.c  | 30 ++++++-------------
  1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms.c b/tools/testing/selftests/bpf/prog_tests/ksyms.c
index b295969b263b..6a86d1f07800 100644
--- a/tools/testing/selftests/bpf/prog_tests/ksyms.c
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms.c
@@ -5,8 +5,6 @@
  #include "test_ksyms.skel.h"
  #include <sys/stat.h>

-static int duration;
-
  void test_ksyms(void)
  {
         const char *btf_path = "/sys/kernel/btf/vmlinux";
@@ -18,43 +16,33 @@ void test_ksyms(void)
         int err;

         err = kallsyms_find("bpf_link_fops", &link_fops_addr);
-       if (CHECK(err == -EINVAL, "kallsyms_fopen", "failed to open: %d\n", errno))
-               return;
-       if (CHECK(err == -ENOENT, "ksym_find", "symbol 'bpf_link_fops' not found\n"))
+       if (!ASSERT_TRUE(err != -EINVAL && err != -ENOENT, "bpf_link_fops"))
                 return;
it's best to keep each individual equality test as ASSERT_EQ(), that
way we get more specific and detailed error (with expected vs actual
value), which here we'll just get "wanted true but got false", and
then we'd have to debug some more which of the conditions it is. So
please keep the original granularity of checks everywhere.

Do you mean

        if (ASSERT_EQ(err, -EINVAL, "..."))
                return;
        if (ASSERT_EQ(err, -ENOENT, "..."))
                return;

This does not really work, for example if err = 0, it will declare test failure.



         err = kallsyms_find("__per_cpu_start", &per_cpu_start_addr);
-       if (CHECK(err == -EINVAL, "kallsyms_fopen", "failed to open: %d\n", errno))
-               return;
-       if (CHECK(err == -ENOENT, "ksym_find", "symbol 'per_cpu_start' not found\n"))
+       if (!ASSERT_TRUE(err != -EINVAL && err != -ENOENT, "__per_cpu_start"))
                 return;

-       if (CHECK(stat(btf_path, &st), "stat_btf", "err %d\n", errno))
+       if (!ASSERT_OK(stat(btf_path, &st), "stat_btf"))
                 return;
         btf_size = st.st_size;

         skel = test_ksyms__open_and_load();
-       if (CHECK(!skel, "skel_open", "failed to open and load skeleton\n"))
+       if (!ASSERT_OK_PTR(skel, "test_ksyms__open_and_load"))
                 return;

         err = test_ksyms__attach(skel);
-       if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
+       if (!ASSERT_OK(err, "test_ksyms__attach"))
                 goto cleanup;

         /* trigger tracepoint */
         usleep(1);

         data = skel->data;
-       CHECK(data->out__bpf_link_fops != link_fops_addr, "bpf_link_fops",
-             "got 0x%llx, exp 0x%llx\n",
-             data->out__bpf_link_fops, link_fops_addr);
-       CHECK(data->out__bpf_link_fops1 != 0, "bpf_link_fops1",
-             "got %llu, exp %llu\n", data->out__bpf_link_fops1, (__u64)0);
-       CHECK(data->out__btf_size != btf_size, "btf_size",
-             "got %llu, exp %llu\n", data->out__btf_size, btf_size);
-       CHECK(data->out__per_cpu_start != per_cpu_start_addr, "__per_cpu_start",
-             "got %llu, exp %llu\n", data->out__per_cpu_start,
-             per_cpu_start_addr);
+       ASSERT_EQ(data->out__bpf_link_fops, link_fops_addr, "bpf_link_fops");
+       ASSERT_EQ(data->out__bpf_link_fops1, 0, "bpf_link_fops1");
+       ASSERT_EQ(data->out__btf_size, btf_size, "btf_size");
+       ASSERT_EQ(data->out__per_cpu_start, per_cpu_start_addr, "__per_cpu_start");

  cleanup:
         test_ksyms__destroy(skel);
--
2.43.0





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux