[PATCH v3 0/3] nolibc: Add dirfd()

[PATCH v3 0/3] nolibc: Add dirfd()

[Daniel Palmer]

Add dirfd() to convert a DIR back to an fd for functions like fstatat()
that take an fd.

Change log:
v3 - Split the internal helper into its own commit.
   - test: just stat one child instead of all of them
   - According to POSIX (https://man7.org/linux/man-pages/man3/dirfd.3.html) errno
     should be EINVAL on failure, so closedir() is now untouched as it is setting EBADF.
     This makes the diff a little cleaner too.
   - Replace i with fd in some places so isn't confused with -1.
   - Due to all of the changes I didn't add trailers from v2.
v2 - Fix the name of the dirfd helper, I debated over removing the helper
     as suggested by Thomas but it seems right to have the logic in a single
     place.
     Add test a basic test to make sure dirfd() actually does convert back
     to the expected fd and using dirfd() with fstatat().

Daniel Palmer (3):
  tools/nolibc: Add internal __nolibc_dirfd() helper
  tools/nolibc: Add dirfd()
  selftests/nolibc: Add test for dirfd()

 tools/include/nolibc/dirent.h                | 37 ++++++++++++++----
 tools/testing/selftests/nolibc/nolibc-test.c | 40 ++++++++++++++++++++
 2 files changed, 70 insertions(+), 7 deletions(-)

-- 
2.53.0

[PATCH v3 1/3] tools/nolibc: Add internal __nolibc_dirfd() helper

[Daniel Palmer]

closedir() and readdir_r() both convert a DIR pointer back into
an fd. Move that logic into a single helper.

The helper does not set errno so that it can be used by
readdir_r() which returns its own error.

Signed-off-by: Daniel Palmer <daniel@thingy.jp>
---
 tools/include/nolibc/dirent.h | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/include/nolibc/dirent.h b/tools/include/nolibc/dirent.h
index 19857369de2a..7666dc69506f 100644
--- a/tools/include/nolibc/dirent.h
+++ b/tools/include/nolibc/dirent.h
@@ -27,6 +27,18 @@ typedef struct {
 	char dummy[1];
 } DIR;
 
+/* Internal dirfd() that does not set errno */
+static __attribute__((unused))
+int __nolibc_dirfd(DIR *dirp)
+{
+	intptr_t fd = ~(intptr_t)dirp;
+
+	if (fd < 0)
+		return -1;
+
+	return fd;
+}
+
 static __attribute__((unused))
 DIR *fdopendir(int fd)
 {
@@ -65,13 +77,13 @@ DIR *opendir(const char *name)
 static __attribute__((unused))
 int closedir(DIR *dirp)
 {
-	intptr_t i = (intptr_t)dirp;
+	int fd = __nolibc_dirfd(dirp);
 
-	if (i >= 0) {
+	if (fd < 0) {
 		SET_ERRNO(EBADF);
 		return -1;
 	}
-	return close(~i);
+	return close(fd);
 }
 
 static __attribute__((unused))
@@ -79,15 +91,13 @@ int readdir_r(DIR *dirp, struct dirent *entry, struct dirent **result)
 {
 	char buf[sizeof(struct linux_dirent64) + NAME_MAX + 1] __nolibc_aligned_as(struct linux_dirent64);
 	struct linux_dirent64 *ldir = (void *)buf;
-	intptr_t i = (intptr_t)dirp;
 	int fd, ret;
 	off_t off;
 
-	if (i >= 0)
+	fd = __nolibc_dirfd(dirp);
+	if (fd < 0)
 		return EBADF;
 
-	fd = ~i;
-
 	ret = _sys_getdents64(fd, ldir, sizeof(buf));
 	if (ret < 0)
 		return -ret;
-- 
2.53.0

[PATCH v3 2/3] tools/nolibc: Add dirfd()

[Daniel Palmer]

This is useful for things like fstatat() on children of a directory
opened with opendir().

Signed-off-by: Daniel Palmer <daniel@thingy.jp>
---
 tools/include/nolibc/dirent.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/include/nolibc/dirent.h b/tools/include/nolibc/dirent.h
index 7666dc69506f..76331ed65560 100644
--- a/tools/include/nolibc/dirent.h
+++ b/tools/include/nolibc/dirent.h
@@ -74,6 +74,19 @@ DIR *opendir(const char *name)
 	return (DIR *)(intptr_t)~fd;
 }
 
+static __attribute__((unused))
+int dirfd(DIR *dirp)
+{
+	int fd = __nolibc_dirfd(dirp);
+
+	if (fd < 0) {
+		SET_ERRNO(EINVAL);
+		return -1;
+	}
+
+	return fd;
+}
+
 static __attribute__((unused))
 int closedir(DIR *dirp)
 {
-- 
2.53.0

[PATCH v3 3/3] selftests/nolibc: Add test for dirfd()

[Daniel Palmer]

Test that open() and dirfd() use the same fd and then use dirfd()
with fstatat() to stat one of its children.

The test only runs for the nolibc build because it uses readdir_r()
which is overridden to force a failure in the libc build.

Signed-off-by: Daniel Palmer <daniel@thingy.jp>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 40 ++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index f3d0d27cdbe1..648a539fb42a 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -859,6 +859,45 @@ 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;
+	}
+
+	/* Get the name of a child of dir */
+	if (readdir_r(dir, &dirent, &result) != 0 || !result) {
+		ret = __LINE__;
+		goto out;
+	}
+
+	/* stat the child relative to dir */
+	if (fstatat(dirfd(dir), dirent.d_name, &st, AT_SYMLINK_NOFOLLOW) != 0)
+		ret = __LINE__;
+
+out:
+	closedir(dir);
+
+	return ret;
+}
+
 int test_getcwd(void)
 {
 	char cwd_syscall[PATH_MAX];
@@ -1764,6 +1803,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;
 		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