Re: [PATCH v2 2/2] selftests/nolibc: Add test for dirfd()

[Thomas Weißschuh]

Hi Daniel,

On 2026-10-07 14:29:13+0900, Daniel Palmer wrote:
> Test that open() and dirfd() use the same fd and then use
> dirfd() with fstatat() to stat each of its children.

In general the series looks good. I have some minor requests,
but can also implement these myself if you prefer.
Also I'd like to push the patches into the next development cycles.

> Signed-off-by: Daniel Palmer <daniel@xxxxxxxxx>
> ---
>  tools/testing/selftests/nolibc/nolibc-test.c | 47 ++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index ed860b0a15a1..82616f2c1ba4 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -858,6 +858,52 @@ static int test_dirent(void)
>  	return 0;
>  }
>  
> +static int test_dirfd(void)
> +{
> +	struct dirent dirent, *result;
> +	int ret = 0, fd;
> +	struct stat st;
> +	DIR *dir;
> +
> +	fd = open("/", O_RDONLY | O_DIRECTORY);
> +	if (fd < 0)
> +		return __LINE__;
> +
> +	dir = fdopendir(fd);
> +	if (!dir) {
> +		close(fd);
> +		return __LINE__;
> +	}
> +
> +	/* Check that dirfd() gives the same fd as open() */
> +	if (dirfd(dir) != fd) {
> +		ret = __LINE__;
> +		goto out;
> +	}
> +
> +	/* Loop through the children of the directory calling fstatat() on each */

What is the advantage of reading all entries here?
We already do something similar in test_dirent().
Reading a single entry should be enough.

> +	while (1) {
> +		if (readdir_r(dir, &dirent, &result) != 0) {
> +			ret = __LINE__;
> +			goto out;
> +		}
> +
> +		if (!result)
> +			break;
> +
> +		if (fstatat(dirfd(dir), dirent.d_name, &st, AT_SYMLINK_NOFOLLOW) != 0) {
> +			ret = __LINE__;
> +			goto out;
> +
> +		}
> +	}
> +
> +out:
> +	closedir(dir);
> +
> +	return ret;
> +}
> +
>  int test_getcwd(void)
>  {
>  	char cwd_syscall[PATH_MAX];
> @@ -1651,6 +1697,7 @@ int run_syscall(int min, int max)
>  		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
>  		CASE_TEST(getdents64_null);   EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
>  		CASE_TEST(directories);       EXPECT_SYSZR(is_nolibc && proc, test_dirent()); break;
> +		CASE_TEST(dirfd);             EXPECT_SYSZR(is_nolibc, test_dirfd()); break;

Could you mention in the commit message why this is specific to nolibc?

>  		CASE_TEST(getrandom);         EXPECT_SYSZR(1, test_getrandom()); break;
>  		CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
>  		CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
> -- 
> 2.53.0
>