Re: [PATCH v6 6/6] selftests: prctl: Add test for long thread names

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `check_name()` function evaluates negative error codes as test successes due to `EXPECT_TRUE()` behavior.
- [Medium] The test fails rather than gracefully skipping on older kernels lacking `PR_SET_EXT_NAME` support.
--

commit 56060d6784146397018fe8a302145c26fb029757
Author: André Almeida <andrealmeid@igalia.com>
Subject: selftests: prctl: Add test for long thread names

This commit adds tests for setting and getting long thread names using
the new extended PR_SET_EXT_NAME and PR_GET_EXT_NAME prctl interfaces. It
also adjusts the check_is_name_correct() function to prevent negative error
codes from being evaluated as successes by EXPECT_TRUE().

> diff --git a/tools/testing/selftests/prctl/set-process-name.c b/tools/testing/selftests/prctl/set-process-name.c
> --- a/tools/testing/selftests/prctl/set-process-name.c
> +++ b/tools/testing/selftests/prctl/set-process-name.c
[ ... ]
> @@ -80,10 +118,15 @@ int check_name(void)
>  TEST(rename_process) {
>  
>  	EXPECT_GE(set_name(CHANGE_NAME), 0);
> -	EXPECT_TRUE(check_is_name_correct(CHANGE_NAME));
> +	EXPECT_FALSE(check_is_name_correct(CHANGE_NAME));
> +
> +	EXPECT_GE(set_ext_name(LONG_NAME), 0);

[Severity: Medium]
Will this test cause a failure rather than gracefully skipping on older
kernels?

If the test runs on a kernel where the PR_SET_EXT_NAME interface is
unsupported, set_ext_name() returns a negative error code like -EINVAL.
The EXPECT_GE assertion will fail the test entirely.

Should this test gracefully skip when run on kernels lacking support for
these new extensions?

> +	EXPECT_FALSE(check_is_ext_name_correct(LONG_NAME));
> +	EXPECT_FALSE(check_is_name_correct(LONG_NAME_CAP));
> +	EXPECT_TRUE(check_name());

[Severity: Medium]
Does this new test invocation suffer from the same logic error being fixed
in check_is_name_correct()?

The commit message explicitly mentions fixing check_is_name_correct() because
EXPECT_TRUE() evaluates negative error codes as success.

While the flawed logic in the check_name() function is a pre-existing issue,
it returns -EIO on file open failure and -errno on prctl failure:

tools/testing/selftests/prctl/set-process-name.c:check_name() {
    ...
    if (!fptr)
        return -EIO;
    ...
    if (res < 0)
        return -errno;
    ...
}

Since this patch introduces a new EXPECT_TRUE(check_name()) call here, won't
any file open or prctl failure be falsely evaluated as a test success? Could
we update check_name() as well to prevent this flaw from propagating?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-tonyk-long_name-v6-0-d70afbf194c5@igalia.com?part=6