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

Ayash-Bera posted 1 patch 1 month, 2 weeks ago
tools/testing/selftests/seccomp/seccomp_bpf.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH] selftests/seccomp: improve clarity of test messages and comments
Posted by Ayash-Bera 1 month, 2 weeks ago
Replace ambiguous language in comments and test descriptions to improve
code readability and make test intentions clearer.

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
Re: [PATCH] selftests/seccomp: improve clarity of test messages and comments
Posted by Kees Cook 1 month, 1 week ago
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