[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <b488b997e1b966f6d35a2767da738fc9a44023a8.1721903630.git.tony.ambardar@gmail.com>
Date: Thu, 25 Jul 2024 03:35:54 -0700
From: Tony Ambardar <tony.ambardar@...il.com>
To: bpf@...r.kernel.org
Cc: Tony Ambardar <tony.ambardar@...il.com>,
linux-kselftest@...r.kernel.org,
netdev@...r.kernel.org,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>,
Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...ichev.me>,
Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>,
Mykola Lysenko <mykolal@...com>,
Shuah Khan <shuah@...nel.org>,
Björn Töpel <bjorn@...nel.org>,
Magnus Karlsson <magnus.karlsson@...el.com>,
Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
Jonathan Lemon <jonathan.lemon@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Yan Zhai <yan@...udflare.com>
Subject: [PATCH bpf-next v1 2/8] selftests/bpf: Fix arg parsing in veristat, test_progs
From: Tony Ambardar <tony.ambardar@...il.com>
Current code parses arguments with strtok_r() using a construct like
char *state = NULL;
while ((next = strtok_r(state ? NULL : input, ",", &state))) {
...
}
where logic assumes the 'state' var can distinguish between first and
subsequent strtok_r() calls, and adjusts parameters accordingly. However,
'state' is strictly internal context for strtok_r() and no such assumptions
are supported in the man page. Moreover, the exact behaviour of 'state'
depends on the libc implementation, making the above code fragile.
Indeed, invoking "./test_progs -t <test_name>" on mips64el/musl will hang,
with the above code in an infinite loop.
Similarly, we see strange behaviour running 'veristat' on mips64el/musl:
$ ./veristat -e file,prog,verdict,insns -C two-ok add-failure
Can't specify more than 9 stats
Rewrite code using a 'for' loop without logic dependent on var 'state', the
same approach already used in cgroup_helpers.c.
Fixes: 61ddff373ffa ("selftests/bpf: Improve by-name subtest selection logic in prog_tests")
Fixes: 394169b079b5 ("selftests/bpf: add comparison mode to veristat")
Fixes: c8bc5e050976 ("selftests/bpf: Add veristat tool for mass-verifying BPF object files")
Signed-off-by: Tony Ambardar <tony.ambardar@...il.com>
---
tools/testing/selftests/bpf/testing_helpers.c | 2 +-
tools/testing/selftests/bpf/veristat.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index ac7c66f4fc7b..2a73b72feb18 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -227,7 +227,7 @@ int parse_test_list(const char *s,
if (!input)
return -ENOMEM;
- while ((test_spec = strtok_r(state ? NULL : input, ",", &state))) {
+ for (test_spec = strtok_r(input, ",", &state); test_spec; test_spec = strtok_r(NULL, ",", &state)) {
err = insert_test(set, test_spec, is_glob_pattern);
if (err)
break;
diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index 11ec1190d582..6808679827ac 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -791,7 +791,7 @@ static int parse_stats(const char *stats_str, struct stat_specs *specs)
if (!input)
return -ENOMEM;
- while ((next = strtok_r(state ? NULL : input, ",", &state))) {
+ for (next = strtok_r(input, ",", &state); next; next = strtok_r(NULL, ",", &state)) {
err = parse_stat(next, specs);
if (err) {
free(input);
@@ -1513,7 +1513,7 @@ static int parse_stats_csv(const char *filename, struct stat_specs *specs,
*stat_cntp += 1;
}
- while ((next = strtok_r(state ? NULL : input, ",\n", &state))) {
+ for (next = strtok_r(input, ",\n", &state); next; next = strtok_r(NULL, ",\n", &state)) {
if (header) {
/* for the first line, set up spec stats */
err = parse_stat(next, specs);
--
2.34.1
Powered by blists - more mailing lists