Re: [PATCH bpf-next 14/16] selftests/bpf: Replace CHECK with ASSERT in test_local_storage

Martin KaFai Lau <martin.lau@xxxxxxxxx> · Tue, 7 Mar 2023 17:24:58 -0800




On 3/7/23 5:15 PM, Andrii Nakryiko wrote:
@@ -60,36 +58,32 @@ static bool check_syscall_operations(int map_fd, int obj_fd)

         /* Looking up an existing element should fail initially */
         err = bpf_map_lookup_elem_flags(map_fd, &obj_fd, &lookup_val, 0);
-       if (CHECK(!err || errno != ENOENT, "bpf_map_lookup_elem",
-                 "err:%d errno:%d\n", err, errno))
+       if (!ASSERT_ERR(err, "bpf_map_lookup_elem") ||
+           !ASSERT_EQ(errno, ENOENT, "errno"))

all libbpf APIs since v1.0 always return actual error number directly,
so no need to check errno anymore, you can simplify this further

                 return false;

         /* Create a new element */
         err = bpf_map_update_elem(map_fd, &obj_fd, &val, BPF_NOEXIST);
-       if (CHECK(err < 0, "bpf_map_update_elem", "err:%d errno:%d\n", err,
-                 errno))
+       if (!ASSERT_OK(err, "bpf_map_update_elem"))
                 return false;

         /* Lookup the newly created element */
         err = bpf_map_lookup_elem_flags(map_fd, &obj_fd, &lookup_val, 0);
-       if (CHECK(err < 0, "bpf_map_lookup_elem", "err:%d errno:%d", err,
-                 errno))
+       if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
                 return false;

         /* Check the value of the newly created element */
-       if (CHECK(lookup_val.value != val.value, "bpf_map_lookup_elem",
-                 "value got = %x errno:%d", lookup_val.value, val.value))
+       if (!ASSERT_EQ(lookup_val.value, val.value, "bpf_map_lookup_elem"))
                 return false;

         err = bpf_map_delete_elem(map_fd, &obj_fd);
-       if (CHECK(err, "bpf_map_delete_elem()", "err:%d errno:%d\n", err,
-                 errno))
+       if (!ASSERT_OK(err, "bpf_map_delete_elem()"))
                 return false;

         /* The lookup should fail, now that the element has been deleted */
         err = bpf_map_lookup_elem_flags(map_fd, &obj_fd, &lookup_val, 0);
-       if (CHECK(!err || errno != ENOENT, "bpf_map_lookup_elem",
-                 "err:%d errno:%d\n", err, errno))
+       if (!ASSERT_ERR(err, "bpf_map_lookup_elem") ||
+           !ASSERT_EQ(errno, ENOENT, "errno"))

same here and probably in other places (I haven't checked everything)

Ack. will simplify.






