Re: [PATCH] selftests/seccomp: improve clarity of test messages and comments

[Kees Cook <kees@kernel.org>]

On Sun, Aug 17, 2025 at 12:12:52PM +0530, Ayash-Bera wrote:
> Replace ambiguous language in comments and test descriptions to improve
> code readability and make test intentions clearer.

Thanks; these are good clarifications. Can you please use checkpatch.pl
and then wrap the long commit string?

-Kees

> 
> Changes made:
> - Make TODO comment more specific about 64-bit vs 32-bit argument
>   handling test requirements  
> - Clarify comment about task termination during syscall execution
> - Replace vague "bad recv()" with specific "invalid recv() with NULL parameter"
> - Replace informal "bad flags" with "invalid flags" for consistency
> 
> These improvements help maintainers and contributors better understand
> the expected test behavior.
> 
> Signed-off-by: Ayash Bera <ayashbera@gmail.com>
> 
> ---
>  tools/testing/selftests/seccomp/seccomp_bpf.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index 61acbd45ffaa..bded07f86a54 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -933,7 +933,7 @@ TEST(KILL_unknown)
>  	ASSERT_EQ(SIGSYS, WTERMSIG(status));
>  }
>  
> -/* TODO(wad) add 64-bit versus 32-bit arg tests. */
> +/* TODO(wad) add tests for 64-bit versus 32-bit argument handling differences. */
>  TEST(arg_out_of_range)
>  {
>  	struct sock_filter filter[] = {
> @@ -3514,7 +3514,7 @@ TEST(user_notification_kill_in_middle)
>  	ASSERT_GE(listener, 0);
>  
>  	/*
> -	 * Check that nothing bad happens when we kill the task in the middle
> +	 * Check that killing the task in the middle of a syscall does not cause crashes or hangs when we kill the task in the middle
>  	 * of a syscall.
>  	 */
>  	pid = fork();
> @@ -3798,7 +3798,7 @@ TEST(user_notification_fault_recv)
>  	if (pid == 0)
>  		exit(syscall(__NR_getppid) != USER_NOTIF_MAGIC);
>  
> -	/* Do a bad recv() */
> +	/* Test invalid recv() with NULL parameter */
>  	EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, NULL), -1);
>  	EXPECT_EQ(errno, EFAULT);
>  
> @@ -4169,13 +4169,13 @@ TEST(user_notification_addfd)
>  	addfd.id = req.id;
>  	addfd.flags = 0x0;
>  
> -	/* Verify bad newfd_flags cannot be set */
> +	/* Verify invalid newfd_flags cannot be set */
>  	addfd.newfd_flags = ~O_CLOEXEC;
>  	EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1);
>  	EXPECT_EQ(errno, EINVAL);
>  	addfd.newfd_flags = O_CLOEXEC;
>  
> -	/* Verify bad flags cannot be set */
> +	/* Verify invalid flags cannot be set */
>  	addfd.flags = 0xff;
>  	EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1);
>  	EXPECT_EQ(errno, EINVAL);
> -- 
> 2.50.1
> 

-- 
Kees Cook