* [PATCH] selftests/bpf: fix array_size.cocci warning:
@ 2021-11-17 13:20 Guo Zhengkui
2021-11-17 22:42 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Guo Zhengkui @ 2021-11-17 13:20 UTC (permalink / raw)
To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Guo Zhengkui, Yucong Sun, Dave Marchevsky,
linux-kselftest, netdev, bpf, linux-kernel
Cc: kernel
Use ARRAY_SIZE() because it uses __must_be_array(arr) to make sure
arr is really an array.
Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
---
.../testing/selftests/bpf/prog_tests/cgroup_attach_override.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
index 356547e849e2..1921c5040d8c 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/kernel.h>
#include <test_progs.h>
#include "cgroup_helpers.h"
@@ -16,10 +17,9 @@ static int prog_load(int verdict)
BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
BPF_EXIT_INSN(),
};
- size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
return bpf_test_load_program(BPF_PROG_TYPE_CGROUP_SKB,
- prog, insns_cnt, "GPL", 0,
+ prog, ARRAY_SIZE(prog), "GPL", 0,
bpf_log_buf, BPF_LOG_BUF_SIZE);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/bpf: fix array_size.cocci warning:
2021-11-17 13:20 [PATCH] selftests/bpf: fix array_size.cocci warning: Guo Zhengkui
@ 2021-11-17 22:42 ` Daniel Borkmann
2021-11-18 7:11 ` [PATCH] selftests/bpf: fix array_size.cocci warning Guo Zhengkui
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2021-11-17 22:42 UTC (permalink / raw)
To: Guo Zhengkui, Shuah Khan, Alexei Starovoitov, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Yucong Sun, Dave Marchevsky, linux-kselftest, netdev,
bpf, linux-kernel
Cc: kernel
On 11/17/21 2:20 PM, Guo Zhengkui wrote:
> Use ARRAY_SIZE() because it uses __must_be_array(arr) to make sure
> arr is really an array.
>
> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
> ---
> .../testing/selftests/bpf/prog_tests/cgroup_attach_override.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
> index 356547e849e2..1921c5040d8c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#include <linux/kernel.h>
> #include <test_progs.h>
No need for the extra include. test_progs.h already includes bpf_util.h, please check
such trivialities before submission. Simple grep would have revealed use of ARRAY_SIZE()
in various places under tools/testing/selftests/bpf/prog_tests/.
> #include "cgroup_helpers.h"
> @@ -16,10 +17,9 @@ static int prog_load(int verdict)
> BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
> BPF_EXIT_INSN(),
> };
> - size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
>
> return bpf_test_load_program(BPF_PROG_TYPE_CGROUP_SKB,
> - prog, insns_cnt, "GPL", 0,
> + prog, ARRAY_SIZE(prog), "GPL", 0,
> bpf_log_buf, BPF_LOG_BUF_SIZE);
There are many more similar occurrences. Please just send one cleanup patch to reduce
churn in the git log.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/bpf: fix array_size.cocci warning
2021-11-17 22:42 ` Daniel Borkmann
@ 2021-11-18 7:11 ` Guo Zhengkui
0 siblings, 0 replies; 6+ messages in thread
From: Guo Zhengkui @ 2021-11-18 7:11 UTC (permalink / raw)
To: daniel
Cc: andrii, ast, bpf, davemarchevsky, guozhengkui, john.fastabend,
kafai, kernel, kpsingh, linux-kernel, linux-kselftest, netdev,
shuah, songliubraving, sunyucong, yhs
> No need for the extra include. test_progs.h already includes bpf_util.h, please check
> such trivialities before submission. Simple grep would have revealed use of ARRAY_SIZE()
> in various places under tools/testing/selftests/bpf/prog_tests/.
Actually, ARRAY_SIZE() in ./include/linux/kernel.h is diffrent from the one defined in bpf_util.h:
./include/linux/kernel.h
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
./tools/testing/selftests/bpf/bpf_util.h
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
__must_be_array() ensures arr is an array, which is better than the one defined in bpf_util.h
> here are many more similar occurrences. Please just send one cleanup patch to reduce churn in the git log.
Yes, I will commit another patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] selftests/bpf: fix array_size.cocci warning
@ 2022-03-08 9:17 Guo Zhengkui
2022-03-08 14:59 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Guo Zhengkui @ 2022-03-08 9:17 UTC (permalink / raw)
To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Guo Zhengkui, Dave Marchevsky, Yucong Sun, Christy Lee,
Delyan Kratunov, Toke Høiland-Jørgensen,
open list:KERNEL SELFTEST FRAMEWORK,
open list:BPF (Safe dynamic programs and tools),
open list:BPF (Safe dynamic programs and tools),
open list
Cc: zhengkui_guo
Fix the array_size.cocci warning in tools/testing/selftests/bpf/
Use `ARRAY_SIZE(arr)` instead of forms like `sizeof(arr)/sizeof(arr[0])`.
syscall.c and test_rdonly_maps.c don't contain header files which
implement ARRAY_SIZE() macro. So I add `#include <linux/kernel.h>`,
in which ARRAY_SIZE(arr) not only calculates the size of `arr`, but also
checks that `arr` is really an array (using __must_be_array(arr)).
Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
---
.../selftests/bpf/prog_tests/cgroup_attach_autodetach.c | 2 +-
.../testing/selftests/bpf/prog_tests/cgroup_attach_multi.c | 2 +-
.../selftests/bpf/prog_tests/cgroup_attach_override.c | 2 +-
tools/testing/selftests/bpf/prog_tests/global_data.c | 6 +++---
tools/testing/selftests/bpf/prog_tests/obj_name.c | 2 +-
tools/testing/selftests/bpf/progs/syscall.c | 3 ++-
tools/testing/selftests/bpf/progs/test_rdonly_maps.c | 3 ++-
tools/testing/selftests/bpf/test_cgroup_storage.c | 2 +-
tools/testing/selftests/bpf/test_lru_map.c | 4 ++--
tools/testing/selftests/bpf/test_sock_addr.c | 6 +++---
tools/testing/selftests/bpf/test_sockmap.c | 4 ++--
11 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
index 858916d11e2e..9367bd2f0ae1 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
@@ -14,7 +14,7 @@ static int prog_load(void)
BPF_MOV64_IMM(BPF_REG_0, 1), /* r0 = 1 */
BPF_EXIT_INSN(),
};
- size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+ size_t insns_cnt = ARRAY_SIZE(prog);
return bpf_test_load_program(BPF_PROG_TYPE_CGROUP_SKB,
prog, insns_cnt, "GPL", 0,
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
index 38b3c47293da..db0b7bac78d1 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
@@ -63,7 +63,7 @@ static int prog_load_cnt(int verdict, int val)
BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
BPF_EXIT_INSN(),
};
- size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+ size_t insns_cnt = ARRAY_SIZE(prog);
int ret;
ret = bpf_test_load_program(BPF_PROG_TYPE_CGROUP_SKB,
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
index 356547e849e2..9421a5b7f4e1 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
@@ -16,7 +16,7 @@ static int prog_load(int verdict)
BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
BPF_EXIT_INSN(),
};
- size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+ size_t insns_cnt = ARRAY_SIZE(prog);
return bpf_test_load_program(BPF_PROG_TYPE_CGROUP_SKB,
prog, insns_cnt, "GPL", 0,
diff --git a/tools/testing/selftests/bpf/prog_tests/global_data.c b/tools/testing/selftests/bpf/prog_tests/global_data.c
index 6fb3d3155c35..027685858925 100644
--- a/tools/testing/selftests/bpf/prog_tests/global_data.c
+++ b/tools/testing/selftests/bpf/prog_tests/global_data.c
@@ -29,7 +29,7 @@ static void test_global_data_number(struct bpf_object *obj, __u32 duration)
{ "relocate .rodata reference", 10, ~0 },
};
- for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
err = bpf_map_lookup_elem(map_fd, &tests[i].key, &num);
CHECK(err || num != tests[i].num, tests[i].name,
"err %d result %llx expected %llx\n",
@@ -58,7 +58,7 @@ static void test_global_data_string(struct bpf_object *obj, __u32 duration)
{ "relocate .bss reference", 4, "\0\0hello" },
};
- for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
err = bpf_map_lookup_elem(map_fd, &tests[i].key, str);
CHECK(err || memcmp(str, tests[i].str, sizeof(str)),
tests[i].name, "err %d result \'%s\' expected \'%s\'\n",
@@ -92,7 +92,7 @@ static void test_global_data_struct(struct bpf_object *obj, __u32 duration)
{ "relocate .data reference", 3, { 41, 0xeeeeefef, 0x2111111111111111ULL, } },
};
- for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
err = bpf_map_lookup_elem(map_fd, &tests[i].key, &val);
CHECK(err || memcmp(&val, &tests[i].val, sizeof(val)),
tests[i].name, "err %d result { %u, %u, %llu } expected { %u, %u, %llu }\n",
diff --git a/tools/testing/selftests/bpf/prog_tests/obj_name.c b/tools/testing/selftests/bpf/prog_tests/obj_name.c
index 6194b776a28b..7093edca6e08 100644
--- a/tools/testing/selftests/bpf/prog_tests/obj_name.c
+++ b/tools/testing/selftests/bpf/prog_tests/obj_name.c
@@ -20,7 +20,7 @@ void test_obj_name(void)
__u32 duration = 0;
int i;
- for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
size_t name_len = strlen(tests[i].name) + 1;
union bpf_attr attr;
size_t ncopy;
diff --git a/tools/testing/selftests/bpf/progs/syscall.c b/tools/testing/selftests/bpf/progs/syscall.c
index e550f728962d..85c6e7849463 100644
--- a/tools/testing/selftests/bpf/progs/syscall.c
+++ b/tools/testing/selftests/bpf/progs/syscall.c
@@ -6,6 +6,7 @@
#include <bpf/bpf_tracing.h>
#include <../../../tools/include/linux/filter.h>
#include <linux/btf.h>
+#include <linux/kernel.h>
char _license[] SEC("license") = "GPL";
@@ -82,7 +83,7 @@ int bpf_prog(struct args *ctx)
static __u64 value = 34;
static union bpf_attr prog_load_attr = {
.prog_type = BPF_PROG_TYPE_XDP,
- .insn_cnt = sizeof(insns) / sizeof(insns[0]),
+ .insn_cnt = ARRAY_SIZE(insns),
};
int ret;
diff --git a/tools/testing/selftests/bpf/progs/test_rdonly_maps.c b/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
index fc8e8a34a3db..ca75aac745a4 100644
--- a/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
+++ b/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
@@ -3,6 +3,7 @@
#include <linux/ptrace.h>
#include <linux/bpf.h>
+#include <linux/kernel.h>
#include <bpf/bpf_helpers.h>
const struct {
@@ -64,7 +65,7 @@ int full_loop(struct pt_regs *ctx)
{
/* prevent compiler to optimize everything out */
unsigned * volatile p = (void *)&rdonly_values.a;
- int i = sizeof(rdonly_values.a) / sizeof(rdonly_values.a[0]);
+ int i = ARRAY_SIZE(rdonly_values.a);
unsigned iters = 0, sum = 0;
/* validate verifier can allow full loop as well */
diff --git a/tools/testing/selftests/bpf/test_cgroup_storage.c b/tools/testing/selftests/bpf/test_cgroup_storage.c
index 5b8314cd77fd..d6a1be4d8020 100644
--- a/tools/testing/selftests/bpf/test_cgroup_storage.c
+++ b/tools/testing/selftests/bpf/test_cgroup_storage.c
@@ -36,7 +36,7 @@ int main(int argc, char **argv)
BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
BPF_EXIT_INSN(),
};
- size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+ size_t insns_cnt = ARRAY_SIZE(prog);
int error = EXIT_FAILURE;
int map_fd, percpu_map_fd, prog_fd, cgroup_fd;
struct bpf_cgroup_storage_key key;
diff --git a/tools/testing/selftests/bpf/test_lru_map.c b/tools/testing/selftests/bpf/test_lru_map.c
index 6e6235185a86..563bbe18c172 100644
--- a/tools/testing/selftests/bpf/test_lru_map.c
+++ b/tools/testing/selftests/bpf/test_lru_map.c
@@ -878,11 +878,11 @@ int main(int argc, char **argv)
assert(nr_cpus != -1);
printf("nr_cpus:%d\n\n", nr_cpus);
- for (f = 0; f < sizeof(map_flags) / sizeof(*map_flags); f++) {
+ for (f = 0; f < ARRAY_SIZE(map_flags); f++) {
unsigned int tgt_free = (map_flags[f] & BPF_F_NO_COMMON_LRU) ?
PERCPU_FREE_TARGET : LOCAL_FREE_TARGET;
- for (t = 0; t < sizeof(map_types) / sizeof(*map_types); t++) {
+ for (t = 0; t < ARRAY_SIZE(map_types); t++) {
test_lru_sanity0(map_types[t], map_flags[f]);
test_lru_sanity1(map_types[t], map_flags[f], tgt_free);
test_lru_sanity2(map_types[t], map_flags[f], tgt_free);
diff --git a/tools/testing/selftests/bpf/test_sock_addr.c b/tools/testing/selftests/bpf/test_sock_addr.c
index f0c8d05ba6d1..f3d5d7ac6505 100644
--- a/tools/testing/selftests/bpf/test_sock_addr.c
+++ b/tools/testing/selftests/bpf/test_sock_addr.c
@@ -723,7 +723,7 @@ static int xmsg_ret_only_prog_load(const struct sock_addr_test *test,
BPF_MOV64_IMM(BPF_REG_0, rc),
BPF_EXIT_INSN(),
};
- return load_insns(test, insns, sizeof(insns) / sizeof(struct bpf_insn));
+ return load_insns(test, insns, ARRAY_SIZE(insns));
}
static int sendmsg_allow_prog_load(const struct sock_addr_test *test)
@@ -795,7 +795,7 @@ static int sendmsg4_rw_asm_prog_load(const struct sock_addr_test *test)
BPF_EXIT_INSN(),
};
- return load_insns(test, insns, sizeof(insns) / sizeof(struct bpf_insn));
+ return load_insns(test, insns, ARRAY_SIZE(insns));
}
static int recvmsg4_rw_c_prog_load(const struct sock_addr_test *test)
@@ -858,7 +858,7 @@ static int sendmsg6_rw_dst_asm_prog_load(const struct sock_addr_test *test,
BPF_EXIT_INSN(),
};
- return load_insns(test, insns, sizeof(insns) / sizeof(struct bpf_insn));
+ return load_insns(test, insns, ARRAY_SIZE(insns));
}
static int sendmsg6_rw_asm_prog_load(const struct sock_addr_test *test)
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 1ba7e7346afb..dfb4f5c0fcb9 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -1786,7 +1786,7 @@ static int populate_progs(char *bpf_file)
i++;
}
- for (i = 0; i < sizeof(map_fd)/sizeof(int); i++) {
+ for (i = 0; i < ARRAY_SIZE(map_fd); i++) {
maps[i] = bpf_object__find_map_by_name(obj, map_names[i]);
map_fd[i] = bpf_map__fd(maps[i]);
if (map_fd[i] < 0) {
@@ -1867,7 +1867,7 @@ static int __test_selftests(int cg_fd, struct sockmap_options *opt)
}
/* Tests basic commands and APIs */
- for (i = 0; i < sizeof(test)/sizeof(struct _test); i++) {
+ for (i = 0; i < ARRAY_SIZE(test); i++) {
struct _test t = test[i];
if (check_whitelist(&t, opt) != 0)
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] selftests/bpf: fix array_size.cocci warning
2022-03-08 9:17 Guo Zhengkui
@ 2022-03-08 14:59 ` Daniel Borkmann
2022-03-08 15:00 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2022-03-08 14:59 UTC (permalink / raw)
To: Guo Zhengkui, Shuah Khan, Alexei Starovoitov, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Dave Marchevsky, Yucong Sun, Christy Lee,
Delyan Kratunov, Toke Høiland-Jørgensen,
open list:KERNEL SELFTEST FRAMEWORK,
open list:BPF (Safe dynamic programs and tools),
open list:BPF (Safe dynamic programs and tools),
open list
Cc: zhengkui_guo
On 3/8/22 10:17 AM, Guo Zhengkui wrote:
> Fix the array_size.cocci warning in tools/testing/selftests/bpf/
>
> Use `ARRAY_SIZE(arr)` instead of forms like `sizeof(arr)/sizeof(arr[0])`.
>
> syscall.c and test_rdonly_maps.c don't contain header files which
> implement ARRAY_SIZE() macro. So I add `#include <linux/kernel.h>`,
> in which ARRAY_SIZE(arr) not only calculates the size of `arr`, but also
> checks that `arr` is really an array (using __must_be_array(arr)).
>
> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
[...]
> diff --git a/tools/testing/selftests/bpf/progs/syscall.c b/tools/testing/selftests/bpf/progs/syscall.c
> index e550f728962d..85c6e7849463 100644
> --- a/tools/testing/selftests/bpf/progs/syscall.c
> +++ b/tools/testing/selftests/bpf/progs/syscall.c
> @@ -6,6 +6,7 @@
> #include <bpf/bpf_tracing.h>
> #include <../../../tools/include/linux/filter.h>
> #include <linux/btf.h>
> +#include <linux/kernel.h>
>
> char _license[] SEC("license") = "GPL";
>
> @@ -82,7 +83,7 @@ int bpf_prog(struct args *ctx)
> static __u64 value = 34;
> static union bpf_attr prog_load_attr = {
> .prog_type = BPF_PROG_TYPE_XDP,
> - .insn_cnt = sizeof(insns) / sizeof(insns[0]),
> + .insn_cnt = ARRAY_SIZE(insns),
> };
> int ret;
>
> diff --git a/tools/testing/selftests/bpf/progs/test_rdonly_maps.c b/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
> index fc8e8a34a3db..ca75aac745a4 100644
> --- a/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
> +++ b/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
> @@ -3,6 +3,7 @@
>
> #include <linux/ptrace.h>
> #include <linux/bpf.h>
> +#include <linux/kernel.h>
> #include <bpf/bpf_helpers.h>
>
> const struct {
> @@ -64,7 +65,7 @@ int full_loop(struct pt_regs *ctx)
> {
> /* prevent compiler to optimize everything out */
> unsigned * volatile p = (void *)&rdonly_values.a;
> - int i = sizeof(rdonly_values.a) / sizeof(rdonly_values.a[0]);
> + int i = ARRAY_SIZE(rdonly_values.a);
> unsigned iters = 0, sum = 0;
>
There's bpf_util.h with ARRAY_SIZE definition which is used in selftests, pls change to
reuse that one.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] selftests/bpf: fix array_size.cocci warning
2022-03-08 14:59 ` Daniel Borkmann
@ 2022-03-08 15:00 ` Daniel Borkmann
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2022-03-08 15:00 UTC (permalink / raw)
To: Guo Zhengkui, Shuah Khan, Alexei Starovoitov, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Dave Marchevsky, Yucong Sun, Christy Lee,
Delyan Kratunov, Toke Høiland-Jørgensen,
open list:KERNEL SELFTEST FRAMEWORK,
open list:BPF (Safe dynamic programs and tools),
open list:BPF (Safe dynamic programs and tools),
open list
Cc: zhengkui_guo
On 3/8/22 3:59 PM, Daniel Borkmann wrote:
> On 3/8/22 10:17 AM, Guo Zhengkui wrote:
>> Fix the array_size.cocci warning in tools/testing/selftests/bpf/
>>
>> Use `ARRAY_SIZE(arr)` instead of forms like `sizeof(arr)/sizeof(arr[0])`.
>>
>> syscall.c and test_rdonly_maps.c don't contain header files which
>> implement ARRAY_SIZE() macro. So I add `#include <linux/kernel.h>`,
>> in which ARRAY_SIZE(arr) not only calculates the size of `arr`, but also
>> checks that `arr` is really an array (using __must_be_array(arr)).
>>
>> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
> [...]
>> diff --git a/tools/testing/selftests/bpf/progs/syscall.c b/tools/testing/selftests/bpf/progs/syscall.c
>> index e550f728962d..85c6e7849463 100644
>> --- a/tools/testing/selftests/bpf/progs/syscall.c
>> +++ b/tools/testing/selftests/bpf/progs/syscall.c
>> @@ -6,6 +6,7 @@
>> #include <bpf/bpf_tracing.h>
>> #include <../../../tools/include/linux/filter.h>
>> #include <linux/btf.h>
>> +#include <linux/kernel.h>
>> char _license[] SEC("license") = "GPL";
>> @@ -82,7 +83,7 @@ int bpf_prog(struct args *ctx)
>> static __u64 value = 34;
>> static union bpf_attr prog_load_attr = {
>> .prog_type = BPF_PROG_TYPE_XDP,
>> - .insn_cnt = sizeof(insns) / sizeof(insns[0]),
>> + .insn_cnt = ARRAY_SIZE(insns),
>> };
>> int ret;
>> diff --git a/tools/testing/selftests/bpf/progs/test_rdonly_maps.c b/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
>> index fc8e8a34a3db..ca75aac745a4 100644
>> --- a/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
>> +++ b/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
>> @@ -3,6 +3,7 @@
>> #include <linux/ptrace.h>
>> #include <linux/bpf.h>
>> +#include <linux/kernel.h>
>> #include <bpf/bpf_helpers.h>
>> const struct {
>> @@ -64,7 +65,7 @@ int full_loop(struct pt_regs *ctx)
>> {
>> /* prevent compiler to optimize everything out */
>> unsigned * volatile p = (void *)&rdonly_values.a;
>> - int i = sizeof(rdonly_values.a) / sizeof(rdonly_values.a[0]);
>> + int i = ARRAY_SIZE(rdonly_values.a);
>> unsigned iters = 0, sum = 0;
>
> There's bpf_util.h with ARRAY_SIZE definition which is used in selftests, pls change to
> reuse that one.
Also, CI failed with your patch:
https://github.com/kernel-patches/bpf/runs/5462211251?check_suite_focus=true
In file included from progs/test_rdonly_maps.c:6:
In file included from /usr/include/linux/kernel.h:5:
/usr/include/linux/sysinfo.h:9:2: error: unknown type name '__kernel_long_t'
__kernel_long_t uptime; /* Seconds since boot */
^
/usr/include/linux/sysinfo.h:10:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t loads[3]; /* 1, 5, and 15 minute load averages */
^
/usr/include/linux/sysinfo.h:11:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t totalram; /* Total usable main memory size */
^
/usr/include/linux/sysinfo.h:12:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t freeram; /* Available memory size */
^
/usr/include/linux/sysinfo.h:13:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t sharedram; /* Amount of shared memory */
^
/usr/include/linux/sysinfo.h:14:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t bufferram; /* Memory used by buffers */
^
/usr/include/linux/sysinfo.h:15:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t totalswap; /* Total swap space size */
^
/usr/include/linux/sysinfo.h:16:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t freeswap; /* swap space still available */
^
/usr/include/linux/sysinfo.h:19:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t totalhigh; /* Total high memory size */
^
/usr/include/linux/sysinfo.h:20:2: error: unknown type name '__kernel_ulong_t'
__kernel_ulong_t freehigh; /* Available high memory size */
^
/usr/include/linux/sysinfo.h:22:22: error: use of undeclared identifier '__kernel_ulong_t'
char _f[20-2*sizeof(__kernel_ulong_t)-sizeof(__u32)]; /* Padding: libc5 uses this.. */
^
progs/test_rdonly_maps.c:68:10: error: implicit declaration of function 'ARRAY_SIZE' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
int i = ARRAY_SIZE(rdonly_values.a);
^
12 errors generated.
make: *** [Makefile:488: /tmp/runner/work/bpf/bpf/tools/testing/selftests/bpf/test_rdonly_maps.o] Error 1
make: *** Waiting for unfinished jobs....
Error: Process completed with exit code 2.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-08 15:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 13:20 [PATCH] selftests/bpf: fix array_size.cocci warning: Guo Zhengkui
2021-11-17 22:42 ` Daniel Borkmann
2021-11-18 7:11 ` [PATCH] selftests/bpf: fix array_size.cocci warning Guo Zhengkui
2022-03-08 9:17 Guo Zhengkui
2022-03-08 14:59 ` Daniel Borkmann
2022-03-08 15:00 ` Daniel Borkmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.