diff mbox series

[bpf-next,1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result

Message ID 20230316000726.1016773-1-martin.lau@linux.dev (mailing list archive)
State Accepted
Commit ed01385c0d78a025bdc72128b7aa7c3309cd5852
Delegated to: BPF
Headers show
Series [bpf-next,1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 20 this patch: 20
netdev/cc_maintainers warning 12 maintainers not CCed: wangyufen@huawei.com mykolal@fb.com song@kernel.org shuah@kernel.org sdf@google.com haoluo@google.com yhs@fb.com john.fastabend@gmail.com kpsingh@kernel.org jolsa@kernel.org zhuyifei@google.com linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 20 this patch: 20
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain }}
bpf/vmtest-bpf-next-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for aarch64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-5 fail Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-8 success Logs for llvm-toolchain
bpf/vmtest-bpf-next-VM_Test-9 success Logs for set-matrix

Commit Message

Martin KaFai Lau March 16, 2023, 12:07 a.m. UTC 
From: Martin KaFai Lau <martin.lau@kernel.org>

In tcp_hdr_options test, it ensures the received tcp hdr option
and the sk local storage have the expected values. It uses memcmp
to check that. Testing the memcmp result with ASSERT_OK is confusing
because ASSERT_OK will print out the errno which is not set.
This patch uses ASSERT_EQ to check for 0 instead.

Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
---
 tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Yonghong Song March 16, 2023, 12:57 a.m. UTC | #1 
On 3/15/23 5:07 PM, Martin KaFai Lau wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
> 
> In tcp_hdr_options test, it ensures the received tcp hdr option
> and the sk local storage have the expected values. It uses memcmp
> to check that. Testing the memcmp result with ASSERT_OK is confusing
> because ASSERT_OK will print out the errno which is not set.
> This patch uses ASSERT_EQ to check for 0 instead.
> 
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>
John Fastabend March 16, 2023, 5:26 a.m. UTC | #2 
Yonghong Song wrote:
> 
> 
> On 3/15/23 5:07 PM, Martin KaFai Lau wrote:
> > From: Martin KaFai Lau <martin.lau@kernel.org>
> > 
> > In tcp_hdr_options test, it ensures the received tcp hdr option
> > and the sk local storage have the expected values. It uses memcmp
> > to check that. Testing the memcmp result with ASSERT_OK is confusing
> > because ASSERT_OK will print out the errno which is not set.
> > This patch uses ASSERT_EQ to check for 0 instead.
> > 
> > Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> 
> Acked-by: Yonghong Song <yhs@fb.com>

Acked-by: John Fastabend <john.fastabend@gmail.com>
patchwork-bot+netdevbpf@kernel.org March 16, 2023, 5:20 p.m. UTC | #3 
Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Wed, 15 Mar 2023 17:07:25 -0700 you wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
> 
> In tcp_hdr_options test, it ensures the received tcp hdr option
> and the sk local storage have the expected values. It uses memcmp
> to check that. Testing the memcmp result with ASSERT_OK is confusing
> because ASSERT_OK will print out the errno which is not set.
> This patch uses ASSERT_EQ to check for 0 instead.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result
    https://git.kernel.org/bpf/bpf-next/c/ed01385c0d78
  - [bpf-next,2/2] selftests/bpf: Fix a fd leak in an error path in network_helpers.c
    https://git.kernel.org/bpf/bpf-next/c/226efec2b0ef

You are awesome, thank you!
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
index 5cf85d0f9827..13bcaeb028b8 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
@@ -151,7 +151,7 @@  static int check_hdr_opt(const struct bpf_test_option *exp,
 			 const struct bpf_test_option *act,
 			 const char *hdr_desc)
 {
-	if (!ASSERT_OK(memcmp(exp, act, sizeof(*exp)), hdr_desc)) {
+	if (!ASSERT_EQ(memcmp(exp, act, sizeof(*exp)), 0, hdr_desc)) {
 		print_option(exp, "expected: ");
 		print_option(act, "  actual: ");
 		return -1;
@@ -169,7 +169,7 @@  static int check_hdr_stg(const struct hdr_stg *exp, int fd,
 		  "map_lookup(hdr_stg_map_fd)"))
 		return -1;
 
-	if (!ASSERT_OK(memcmp(exp, &act, sizeof(*exp)), stg_desc)) {
+	if (!ASSERT_EQ(memcmp(exp, &act, sizeof(*exp)), 0, stg_desc)) {
 		print_hdr_stg(exp, "expected: ");
 		print_hdr_stg(&act, "  actual: ");
 		return -1;