Skip to contentSkip to conversation

selftests/bpf: Preserve errno in test_progs CHECK macros

PATCH Applied Andrey Ignatov 3 participants 3 messages Dec 20, 2019 – Dec 21, 2019

Subsystems (3)
Kselftest BPF BPF selftests
lore
Share & tools
lore

Andrey Ignatov

author
#0 lore raw

PATCH 1/1 selftests/bpf: Preserve errno in test_progs CHECK macros

It's follow-up for discussion [1]

CHECK and CHECK_FAIL macros in test_progs.h can affect errno in some circumstances, e.g. if some code accidentally closes stdout. It makes checking errno in patterns like this unreliable:

	if (CHECK(!bpf_prog_attach_xattr(...), "tag", "msg"))
		goto err;
	CHECK_FAIL(errno != ENOENT);

, since by CHECK_FAIL time errno could be affected not only by bpf_prog_attach_xattr but by CHECK as well.

Fix it by saving and restoring errno in the macros. There is no "Fixes" tag since no problems were discovered yet and it's rather precaution.

test_progs was run with this change and no difference was identified.

[1] https://lore.kernel.org/bpf/[email protected]/

Signed-off-by Andrey Ignatov <[email protected]>
---
 tools/testing/selftests/bpf/test_progs.h | 4 ++++
 1 file changed, 4 insertions(+)
tools/testing/selftests/bpf/test_progs.h +4 -0
index 8477df835979..de1fdaa4e7b4 100644
@@ -100,6 +100,7 @@ extern struct ipv6_packet pkt_v6;
 
 #define _CHECK(condition, tag, duration, format...) ({			\
 	int __ret = !!(condition);					\
+	int __save_errno = errno;					\
 	if (__ret) {							\
 		test__fail();						\
 		printf("%s:FAIL:%s ", __func__, tag);			\

@@ -108,15 +109,18 @@ extern struct ipv6_packet pkt_v6;
 		printf("%s:PASS:%s %d nsec\n",				\
 		       __func__, tag, duration);			\
 	}								\
+	errno = __save_errno;						\
 	__ret;								\
 })
 
 #define CHECK_FAIL(condition) ({					\
 	int __ret = !!(condition);					\
+	int __save_errno = errno;					\
 	if (__ret) {							\
 		test__fail();						\
 		printf("%s:FAIL:%d\n", __func__, __LINE__);		\
 	}								\
+	errno = __save_errno;						\
 	__ret;								\
 })
Signature · 1 line
2.17.1
Acked-by on v1 1/1
#1 lore raw
Quoted 17 lines · Andrey Ignatov

It's follow-up for discussion [1]

CHECK and CHECK_FAIL macros in test_progs.h can affect errno in some circumstances, e.g. if some code accidentally closes stdout. It makes checking errno in patterns like this unreliable:

        if (CHECK(!bpf_prog_attach_xattr(...), "tag", "msg"))
                goto err;
        CHECK_FAIL(errno != ENOENT);

, since by CHECK_FAIL time errno could be affected not only by bpf_prog_attach_xattr but by CHECK as well.

Fix it by saving and restoring errno in the macros. There is no "Fixes" tag since no problems were discovered yet and it's rather precaution.

Quoted 6 lines

test_progs was run with this change and no difference was identified.

[1] https://lore.kernel.org/bpf/[email protected]/

Signed-off-by Andrey Ignatov <[email protected]>

---

LGTM. Doesn't hurt for sure.

Acked-by Andrii Nakryiko <[email protected]>
Quoted 3 lines
 tools/testing/selftests/bpf/test_progs.h | 4 ++++
 1 file changed, 4 insertions(+)
Applied
#2 lore raw
In reply to Andrey Ignatov
Quoted 14 lines · Andrey Ignatov

It's follow-up for discussion [1]

CHECK and CHECK_FAIL macros in test_progs.h can affect errno in some circumstances, e.g. if some code accidentally closes stdout. It makes checking errno in patterns like this unreliable:

        if (CHECK(!bpf_prog_attach_xattr(...), "tag", "msg"))
                goto err;
        CHECK_FAIL(errno != ENOENT);

, since by CHECK_FAIL time errno could be affected not only by bpf_prog_attach_xattr but by CHECK as well.

Quoted 8 lines

Fix it by saving and restoring errno in the macros. There is no "Fixes" tag since no problems were discovered yet and it's rather precaution.

test_progs was run with this change and no difference was identified.

[1] https://lore.kernel.org/bpf/[email protected]/

Signed-off-by Andrey Ignatov <[email protected]>

Applied. Thanks