[osv-dev] [PATCH] vfs: implement renameat()

2022-05-20 Thread Waldemar Kozaczuk
This patch implements the renameat() function and enhances
tst-rename.cc to unit test it.

This patch also exposes renameat as a syscall.

#Refs 1188

Signed-off-by: Waldemar Kozaczuk 
---
 exported_symbols/osv_ld-musl.so.1.symbols |  1 +
 exported_symbols/osv_libc.so.6.symbols|  1 +
 fs/vfs/main.cc| 30 ++
 linux.cc  |  1 +
 tests/tst-rename.cc   | 48 ++-
 5 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/exported_symbols/osv_ld-musl.so.1.symbols 
b/exported_symbols/osv_ld-musl.so.1.symbols
index 3db22e0d..6d88fda4 100644
--- a/exported_symbols/osv_ld-musl.so.1.symbols
+++ b/exported_symbols/osv_ld-musl.so.1.symbols
@@ -868,6 +868,7 @@ remquo
 remquof
 remquol
 rename
+renameat
 res_init
 res_mkquery
 rewind
diff --git a/exported_symbols/osv_libc.so.6.symbols 
b/exported_symbols/osv_libc.so.6.symbols
index e29059bb..b27fbba1 100644
--- a/exported_symbols/osv_libc.so.6.symbols
+++ b/exported_symbols/osv_libc.so.6.symbols
@@ -696,6 +696,7 @@ register_printf_specifier
 register_printf_type
 remove
 rename
+renameat
 __res_init
 rewind
 rewinddir
diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
index bdedc6c6..76e1ee0e 100644
--- a/fs/vfs/main.cc
+++ b/fs/vfs/main.cc
@@ -1043,6 +1043,36 @@ int rename(const char *oldpath, const char *newpath)
 return -1;
 }
 
+OSV_LIBC_API
+int renameat(int olddirfd, const char *oldpath,
+ int newdirfd, const char *newpath)
+{
+if (!oldpath || !newpath) {
+errno = EINVAL;
+return -1;
+}
+
+if (newpath[0] == '/' || newdirfd == AT_FDCWD) {
+return vfs_fun_at2(olddirfd, oldpath, [newpath](const char *path) {
+return rename(path, newpath);
+});
+} else {
+char absolute_newpath[PATH_MAX];
+auto error = vfs_fun_at(newdirfd, newpath, [_newpath](const 
char *absolute_path) {
+strcpy(absolute_newpath, absolute_path);
+return 0;
+});
+
+if (error) {
+return error;
+} else {
+return vfs_fun_at2(olddirfd, oldpath, [absolute_newpath](const 
char *path) {
+return rename(path, absolute_newpath);
+});
+}
+}
+}
+
 TRACEPOINT(trace_vfs_chdir, "\"%s\"", const char*);
 TRACEPOINT(trace_vfs_chdir_ret, "");
 TRACEPOINT(trace_vfs_chdir_err, "%d", int);
diff --git a/linux.cc b/linux.cc
index f60489e3..1fa01326 100644
--- a/linux.cc
+++ b/linux.cc
@@ -516,6 +516,7 @@ OSV_LIBC_API long syscall(long number, ...)
 SYSCALL3(unlinkat, int, const char *, int);
 SYSCALL3(symlinkat, const char *, int, const char *);
 SYSCALL3(sys_getdents64, int, void *, size_t);
+SYSCALL4(renameat, int, const char *, int, const char *);
 }
 
 debug_always("syscall(): unimplemented system call %d\n", number);
diff --git a/tests/tst-rename.cc b/tests/tst-rename.cc
index 722fdc4c..0668bf77 100644
--- a/tests/tst-rename.cc
+++ b/tests/tst-rename.cc
@@ -73,6 +73,9 @@ static void assert_rename_fails(const fs::path , const 
fs::path , std::v
 BOOST_TEST_MESSAGE("Renaming " + src.string() + " to " + dst.string());
 BOOST_REQUIRE(rename(src.c_str(), dst.c_str()) == -1);
 assert_one_of(errno, errnos);
+BOOST_TEST_MESSAGE("Renaming(at) " + src.string() + " to " + dst.string());
+BOOST_REQUIRE(renameat(AT_FDCWD, src.c_str(), AT_FDCWD, dst.c_str()) == 
-1);
+assert_one_of(errno, errnos);
 }
 
 static void assert_renames(const fs::path src, const fs::path dst)
@@ -82,11 +85,27 @@ static void assert_renames(const fs::path src, const 
fs::path dst)
 BOOST_REQUIRE_MESSAGE(result == 0, fmt("Rename should succeed, errno=%d") 
% errno);
 }
 
-static void test_rename(const fs::path , const fs::path )
+static void assert_renames_at(const fs::path src, const fs::path dst)
+{
+BOOST_TEST_MESSAGE("Renaming " + src.string() + " to " + dst.string());
+auto src_dir_fd = open(src.parent_path().c_str(), O_DIRECTORY);
+auto dst_dir_fd = open(dst.parent_path().c_str(), O_DIRECTORY);
+int result = renameat(src_dir_fd, src.filename().c_str(), dst_dir_fd, 
dst.filename().c_str());
+BOOST_REQUIRE_MESSAGE(result == 0, fmt("Renameat should succeed, 
errno=%d") % errno);
+close(src_dir_fd);
+close(dst_dir_fd);
+}
+
+static void test_rename(const fs::path , const fs::path , bool at = 
false)
 {
 prepare_file(src);
 
-assert_renames(src, dst);
+if (at) {
+assert_renames_at(src, dst);
+}
+else {
+assert_renames(src, dst);
+}
 
 check_file(dst);
 BOOST_CHECK_MESSAGE(!fs::exists(src), "Old file should not exist");
@@ -136,6 +155,10 @@ BOOST_AUTO_TEST_CASE(test_renaming_in_the_same_directory)
 dir / "file1",
 dir / "file2");
 
+test_rename(
+dir / "file1",
+dir / "file2", true);
+
 test_rename_from_open_file(
 dir / "file1",
 dir / "file2");

[osv-dev] [PATCH] multibyte: add __mbstowcs_chk

2022-05-20 Thread Waldemar Kozaczuk
Signed-off-by: Waldemar Kozaczuk 
---
 Makefile|  1 +
 libc/multibyte/__mbstowcs_chk.c | 17 +
 2 files changed, 18 insertions(+)
 create mode 100644 libc/multibyte/__mbstowcs_chk.c

diff --git a/Makefile b/Makefile
index 3e87a16d..19a4571b 100644
--- a/Makefile
+++ b/Makefile
@@ -1451,6 +1451,7 @@ libc += multibyte/__mbsnrtowcs_chk.o
 musl += multibyte/mbsrtowcs.o
 libc += multibyte/__mbsrtowcs_chk.o
 musl += multibyte/mbstowcs.o
+libc += multibyte/__mbstowcs_chk.o
 musl += multibyte/mbtowc.o
 musl += multibyte/wcrtomb.o
 musl += multibyte/wcsnrtombs.o
diff --git a/libc/multibyte/__mbstowcs_chk.c b/libc/multibyte/__mbstowcs_chk.c
new file mode 100644
index ..fcc390df
--- /dev/null
+++ b/libc/multibyte/__mbstowcs_chk.c
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2022 Waldemar Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#include 
+#include 
+
+size_t __mbstowcs_chk(wchar_t *restrict dest, const char *restrict src, size_t 
n, size_t dstlen)
+{
+if (n > dstlen) {
+_chk_fail("mbstowcs");
+}
+return mbstowcs(dest, src, n);
+}
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/20220520193257.146906-1-jwkozaczuk%40gmail.com.


[osv-dev] [PATCH V2] syscall: implement getdents64

2022-05-20 Thread Waldemar Kozaczuk
V2: The only difference is removed delete_dir() function
was accidentally left from previous attempts to implement this
syscall.

It looks like the golang apps that need to iterate over entries
in a directory use a system call getdents64 which is documented
in https://man7.org/linux/man-pages/man2/getdents.2.html. Normally
this functionality is provided by the libc functions like opendir(),
readdir(), etc which actually do delegate to getdents64. Go is known
of bypassing libc in such cases.

So this patch implements the syscall getdents64 by adding a utility
function to VFS main.cc that is then called by syscall in linux.cc.
For details of how this function works please look at the comments.

This patch also adds a unit test to verify this syscall works.

Refs #1188

Signed-off-by: Waldemar Kozaczuk 
---
 fs/vfs/main.cc |  65 
 linux.cc   |   4 ++
 modules/tests/Makefile |   2 +-
 tests/tst-getdents.cc  | 111 +
 4 files changed, 181 insertions(+), 1 deletion(-)
 create mode 100644 tests/tst-getdents.cc

diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
index 8e3d4e5e..bdedc6c6 100644
--- a/fs/vfs/main.cc
+++ b/fs/vfs/main.cc
@@ -790,6 +790,71 @@ int readdir64_r(DIR *dir, struct dirent64 *entry,
 extern "C" OSV_LIBC_API
 struct dirent *readdir64(DIR *dir) __attribute__((alias("readdir")));
 
+struct linux_dirent64 {
+u64d_ino;
+s64d_off;
+unsigned short d_reclen;
+unsigned char  d_type;
+char   d_name[];
+};
+
+#undef getdents64
+extern "C"
+ssize_t sys_getdents64(int fd, void *dirp, size_t count)
+{
+auto *dir = fdopendir(fd);
+if (dir) {
+// We have verified that fd points to a valid directory
+// but we do NOT need the DIR handle so just delete it
+delete dir;
+
+struct file *fp;
+int error = fget(fd, );
+if (error) {
+errno = error;
+return -1;
+}
+
+size_t bytes_read = 0;
+off_t last_off = -1;
+errno = 0;
+
+// Iterate over as many entries as there is space in the buffer
+// by directly calling sys_readdir()
+struct dirent entry;
+while ((error = sys_readdir(fp, )) == 0) {
+auto rec_len = offsetof(linux_dirent64, d_name) + 
strlen(entry.d_name) + 1;
+if (rec_len <= count) {
+auto *ldirent = static_cast(dirp + 
bytes_read);
+ldirent->d_ino = entry.d_ino;
+ldirent->d_off = entry.d_off;
+ldirent->d_type = entry.d_type;
+strcpy(ldirent->d_name, entry.d_name);
+ldirent->d_reclen = rec_len;
+count -= rec_len;
+bytes_read += rec_len;
+last_off = entry.d_off;
+} else {
+if (last_off >= 0)
+sys_seekdir(fp, last_off);
+break;
+}
+}
+
+fdrop(fp);
+
+if (error && error != ENOENT) {
+errno = error;
+return -1;
+} else {
+errno = 0;
+return bytes_read;
+}
+} else {
+return -1;
+}
+}
+
 OSV_LIBC_API
 void rewinddir(DIR *dirp)
 {
diff --git a/linux.cc b/linux.cc
index 85c08981..f60489e3 100644
--- a/linux.cc
+++ b/linux.cc
@@ -424,6 +424,9 @@ static int tgkill(int tgid, int tid, int sig)
 return -1;
 }
 
+#define __NR_sys_getdents64 __NR_getdents64
+extern "C" ssize_t sys_getdents64(int fd, void *dirp, size_t count);
+
 OSV_LIBC_API long syscall(long number, ...)
 {
 // Save FPU state and restore it at the end of this function
@@ -512,6 +515,7 @@ OSV_LIBC_API long syscall(long number, ...)
 SYSCALL2(statfs, const char *, struct statfs *);
 SYSCALL3(unlinkat, int, const char *, int);
 SYSCALL3(symlinkat, const char *, int, const char *);
+SYSCALL3(sys_getdents64, int, void *, size_t);
 }
 
 debug_always("syscall(): unimplemented system call %d\n", number);
diff --git a/modules/tests/Makefile b/modules/tests/Makefile
index ca489341..e462ebc8 100644
--- a/modules/tests/Makefile
+++ b/modules/tests/Makefile
@@ -133,7 +133,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so 
tst-bsd-evh.so \
tst-getopt.so tst-getopt-pie.so tst-non-pie.so tst-semaphore.so \
tst-elf-init.so tst-realloc.so tst-setjmp.so \
libtls.so libtls_gold.so tst-tls.so tst-tls-gold.so tst-tls-pie.so \
-   tst-sigaction.so tst-syscall.so tst-ifaddrs.so
+   tst-sigaction.so tst-syscall.so tst-ifaddrs.so tst-getdents.so
 #  libstatic-thread-variable.so tst-static-thread-variable.so \
 
 #TODO For now let us disable these tests for aarch64 until
diff --git a/tests/tst-getdents.cc b/tests/tst-getdents.cc
new file mode 100644
index ..5803aaeb
--- /dev/null
+++ b/tests/tst-getdents.cc
@@ -0,0 +1,111 @@
+#include  /* Defines DT_* constants */
+#include 
+#include 

[osv-dev] [PATCH] syscall: implement getcwd

2022-05-20 Thread Waldemar Kozaczuk
#Refs 1188

Signed-off-by: Waldemar Kozaczuk 
---
 linux.cc | 15 +++
 tests/tst-syscall.cc |  7 +++
 2 files changed, 22 insertions(+)

diff --git a/linux.cc b/linux.cc
index dd0dabd1..85c08981 100644
--- a/linux.cc
+++ b/linux.cc
@@ -366,6 +366,20 @@ static int sys_exit_group(int ret)
 return 0;
 }
 
+#define __NR_sys_getcwd __NR_getcwd
+static long sys_getcwd(char *buf, unsigned long size)
+{
+if (!buf) {
+errno = EINVAL;
+return -1;
+}
+auto ret = getcwd(buf, size);
+if (!ret) {
+return -1;
+}
+return strlen(ret) + 1;
+}
+
 #define __NR_sys_ioctl __NR_ioctl
 //
 // We need to define explicit sys_ioctl that takes these 3 parameters to 
conform
@@ -482,6 +496,7 @@ OSV_LIBC_API long syscall(long number, ...)
 SYSCALL2(nanosleep, const struct timespec*, struct timespec *);
 SYSCALL4(fstatat, int, const char *, struct stat *, int);
 SYSCALL1(sys_exit_group, int);
+SYSCALL2(sys_getcwd, char *, unsigned long);
 SYSCALL4(readlinkat, int, const char *, char *, size_t);
 SYSCALL0(getpid);
 SYSCALL3(set_mempolicy, int, unsigned long *, unsigned long);
diff --git a/tests/tst-syscall.cc b/tests/tst-syscall.cc
index 12722f1b..0fbd9c35 100644
--- a/tests/tst-syscall.cc
+++ b/tests/tst-syscall.cc
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -117,6 +118,12 @@ int main(int argc, char **argv)
 
 assert(close(fd) == 0);
 
+assert(chdir("/proc") == 0);
+unsigned long size = 4096;
+char path[size];
+assert(syscall(__NR_getcwd, path, size) == 6);
+assert(strcmp("/proc", path) == 0);
+
 // test that unknown system call results in a ENOSYS (see issue #757)
 expect_errno_l(syscall(999), ENOSYS);
 
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/20220520191546.141775-1-jwkozaczuk%40gmail.com.


[osv-dev] [PATCH V2] vfs: implement symlinkat

2022-05-20 Thread Waldemar Kozaczuk
V2: The implementation uses vfs_fun_at2() instead of vfs_fun_at()
to further simplify code. We also expose symlinkat though syscall.

This patch implements the symlinkat() function and enhances
tst-symlink.cc to unit test it.

#Refs 1188

Signed-off-by: Waldemar Kozaczuk 
---
 exported_symbols/osv_ld-musl.so.1.symbols |  1 +
 exported_symbols/osv_libc.so.6.symbols|  1 +
 fs/vfs/main.cc| 13 +
 linux.cc  |  1 +
 tests/tst-symlink.cc  | 18 +++---
 5 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/exported_symbols/osv_ld-musl.so.1.symbols 
b/exported_symbols/osv_ld-musl.so.1.symbols
index f1c61a3f..3db22e0d 100644
--- a/exported_symbols/osv_ld-musl.so.1.symbols
+++ b/exported_symbols/osv_ld-musl.so.1.symbols
@@ -1081,6 +1081,7 @@ swab
 swprintf
 swscanf
 symlink
+symlinkat
 sync
 syscall
 sysconf
diff --git a/exported_symbols/osv_libc.so.6.symbols 
b/exported_symbols/osv_libc.so.6.symbols
index 7ae57c38..e29059bb 100644
--- a/exported_symbols/osv_libc.so.6.symbols
+++ b/exported_symbols/osv_libc.so.6.symbols
@@ -887,6 +887,7 @@ swprintf
 __swprintf_chk
 swscanf
 symlink
+symlinkat
 sync
 syscall
 sysconf
diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
index 9b2e2c02..8e3d4e5e 100644
--- a/fs/vfs/main.cc
+++ b/fs/vfs/main.cc
@@ -1132,6 +1132,19 @@ int symlink(const char *oldpath, const char *newpath)
 return 0;
 }
 
+OSV_LIBC_API
+int symlinkat(const char *oldpath, int newdirfd, const char *newpath)
+{
+if (!oldpath) {
+errno = EINVAL;
+return -1;
+}
+
+return vfs_fun_at2(newdirfd, newpath, [oldpath](const char * path) {
+return symlink(oldpath, path);
+});
+}
+
 TRACEPOINT(trace_vfs_unlink, "\"%s\"", const char*);
 TRACEPOINT(trace_vfs_unlink_ret, "");
 TRACEPOINT(trace_vfs_unlink_err, "%d", int);
diff --git a/linux.cc b/linux.cc
index 5c271df1..dd0dabd1 100644
--- a/linux.cc
+++ b/linux.cc
@@ -496,6 +496,7 @@ OSV_LIBC_API long syscall(long number, ...)
 SYSCALL3(lseek, int, off_t, int);
 SYSCALL2(statfs, const char *, struct statfs *);
 SYSCALL3(unlinkat, int, const char *, int);
+SYSCALL3(symlinkat, const char *, int, const char *);
 }
 
 debug_always("syscall(): unimplemented system call %d\n", number);
diff --git a/tests/tst-symlink.cc b/tests/tst-symlink.cc
index 978cfda3..1322e79e 100644
--- a/tests/tst-symlink.cc
+++ b/tests/tst-symlink.cc
@@ -25,6 +25,9 @@
 
 #define N1"f1"
 #define N2"f2_AAA"
+#define N2B   "f2_BBB"
+#define N2B   "f2_BBB"
+#define N2C   "f2_CCC"
 #define N3"f3"
 #define N4"f4"
 #define N5"f5"
@@ -91,6 +94,8 @@ int main(int argc, char **argv)
 #endif
 
 report(chdir(TESTDIR) == 0, "chdir");
+auto test_dir = opendir(TESTDIR);
+report(test_dir, "opendir");
 
 /*
  * test to check
@@ -115,6 +120,10 @@ int main(int argc, char **argv)
 #else
 report(symlink(N1, N2) == 0, "symlink");
 report(search_dir(TESTDIR, N2) == true, "search dir");
+report(symlinkat(N1, dirfd(test_dir), N2B) == 0, "symlinkat");
+report(search_dir(TESTDIR, N2B) == true, "search dir N2B");
+report(symlinkat(N1, AT_FDCWD, N2C) == 0, "symlinkat");
+report(search_dir(TESTDIR, N2C) == true, "search dir N2B");
 #endif
 
 #if defined(READ_ONLY_FS)
@@ -125,6 +134,8 @@ int main(int argc, char **argv)
 #else
 report(access(N1, R_OK | W_OK) == 0, "access");
 report(access(N2, R_OK | W_OK) == 0, "access");
+report(access(N2B, R_OK | W_OK) == 0, "access");
+report(access(N2C, R_OK | W_OK) == 0, "access");
 #endif
 
 rc = readlink(N2, path, sizeof(path));
@@ -157,6 +168,8 @@ int main(int argc, char **argv)
 error = errno;
 report(rc < 0 && errno == ENOENT, "ENOENT expected");
 report(unlink(N2) == 0, "unlink");
+report(unlinkat(dirfd(test_dir),N2B,0) == 0, "unlinkat");
+report(unlinkat(dirfd(test_dir),N2C,0) == 0, "unlinkat");
 
 /*
  * IO Tests 1: write(file), read(symlink), truncate(symlink)
@@ -365,8 +378,6 @@ int main(int argc, char **argv)
 report(search_dir(D2, N5) == true, "Symlink search");
 
 report(rename(D2, D3) == 0, "rename(d2, d3)");
-auto test_dir = opendir(TESTDIR);
-report(test_dir, "opendir");
 rc = readlinkat(dirfd(test_dir), D3, path, sizeof(path));
 report(rc >= 0, "readlinkat");
 path[rc] = 0;
@@ -381,7 +392,6 @@ int main(int argc, char **argv)
 report(rc >= 0, "readlinkat");
 path[rc] = 0;
 report(strcmp(path, D1) == 0, "readlinkat path");
-report(closedir(test_dir) == 0, "closedir(test_dir)");
 rc = readlink(D3, path, sizeof(path));
 report(rc >= 0, "readlink");
 path[rc] = 0;
@@ -399,6 +409,8 @@ int main(int argc, char **argv)
 report(rmdir(D4) == 0, "rmdir");
 #endif
 
+report(closedir(test_dir) == 0, "closedir(test_dir)");
+
 #if defined(READ_ONLY_FS)
 report(-1 == rmdir(TESTDIR) && errno == ENOTEMPTY, "rmdir");
 #else
-- 
2.34.1

-- 
You 

[osv-dev] [PATCH V2] unlinkat: fill the gaps in the implementation

2022-05-20 Thread Waldemar Kozaczuk
V2: The implementation uses vfs_fun_at2() instead of vfs_fun_at()
to further simplify code. We also expose unlinkat though syscall.

This patch enhances the unlinkat() implementation to handle
the AT_FDCWD dirfd and AT_REMOVEDIR flags. 

We also enhance tst-remove.cc to test unlinkat.

#Refs 1188

Signed-off-by: Waldemar Kozaczuk 
---
 fs/vfs/main.cc  | 12 +++-
 linux.cc|  1 +
 tests/tst-remove.cc | 18 --
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
index 1b7eb588..9b2e2c02 100644
--- a/fs/vfs/main.cc
+++ b/fs/vfs/main.cc
@@ -1164,11 +1164,13 @@ int unlink(const char *pathname)
 OSV_LIBC_API
 int unlinkat(int dirfd, const char *pathname, int flags)
 {
-//TODO: Really implement it
-if (dirfd != AT_FDCWD || flags) {
-UNIMPLEMENTED("unlinkat() with non-zero flags or dirfd != AT_FDCWD");
-}
-return unlink(pathname);
+return vfs_fun_at2(dirfd, pathname, [flags](const char *path) {
+if (flags & AT_REMOVEDIR) {
+return rmdir(path);
+} else {
+return unlink(path);
+}
+});
 }
 
 TRACEPOINT(trace_vfs_stat, "\"%s\" %p", const char*, struct stat*);
diff --git a/linux.cc b/linux.cc
index c9b6b7b6..5c271df1 100644
--- a/linux.cc
+++ b/linux.cc
@@ -495,6 +495,7 @@ OSV_LIBC_API long syscall(long number, ...)
 SYSCALL0(getuid);
 SYSCALL3(lseek, int, off_t, int);
 SYSCALL2(statfs, const char *, struct statfs *);
+SYSCALL3(unlinkat, int, const char *, int);
 }
 
 debug_always("syscall(): unimplemented system call %d\n", number);
diff --git a/tests/tst-remove.cc b/tests/tst-remove.cc
index 6851cba0..fdf4037d 100644
--- a/tests/tst-remove.cc
+++ b/tests/tst-remove.cc
@@ -42,6 +42,8 @@ bool do_expect(T actual, T expected, const char *actuals, 
const char *expecteds,
 int main(int argc, char **argv)
 {
 expect(mkdir("/tmp/tst-remove", 0777), 0);
+auto tst_remove_dir = open("/tmp/tst-remove", O_DIRECTORY);
+expect(tst_remove_dir != -1, true);
 
 /* test unlink() **/
 // unlink() non-existant file returns ENOENT
@@ -79,12 +81,24 @@ int main(int argc, char **argv)
 expect_errno(rmdir("/tmp/tst-remove/f"), ENOTDIR);
 expect(unlink("/tmp/tst-remove/f"), 0);
 
-/* test remove() ***/
-// TODO...
+/* test unlinkat() ***/
+expect(mknod("/tmp/tst-remove/u", 0777|S_IFREG, 0), 0);
+expect(unlinkat(tst_remove_dir, "u", 0), 0);
 
+expect(mknod("/tmp/tst-remove/u2", 0777|S_IFREG, 0), 0);
+expect(chdir("/tmp/tst-remove"), 0);
+expect(unlinkat(AT_FDCWD, "u2", 0), 0);
+
+expect(mkdir("/tmp/tst-remove/ud", 0777), 0);
+expect(unlinkat(tst_remove_dir, "ud", AT_REMOVEDIR), 0);
+
+expect(mkdir("/tmp/tst-remove/ud2", 0777), 0);
+expect(chdir("/tmp/tst-remove"), 0);
+expect(unlinkat(AT_FDCWD, "ud2", AT_REMOVEDIR), 0);
 
 // Finally remove the temporary directory (assumes the above left
 // nothing in it)
+expect(close(tst_remove_dir), 0);
 expect(rmdir("/tmp/tst-remove"), 0);
 
 
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/20220520185638.136645-1-jwkozaczuk%40gmail.com.


[osv-dev] [PATCH V2] vfs: refactor the *at() functions

2022-05-20 Thread Waldemar Kozaczuk
Comparing to the 1st version, this one adds another helper
function - vfs_fun_at2() - which calls supplied lambda if
dirfd == AT_FDCWD or pathname is an absolute path and otherwise
delegates to vfs_fun_at(). It also checks if pathname is not null.
The __fxstatat() and futimesat() call vfs_fun_at() directly as their
logic is slightly different.

The VFS functions like openat(), faccessat() and others alike
take a directory descriptor argument intended to make a filesystem
action happen relative to that directory.

The implemetations of those function repeat almost the same code
over and over. So this patch makes vfs more DRY by introducing 
a helper function - vfs_fun_at() - which implements common
logic and executed a lambda function specific to given
VFS action.

This patch also adds some unit tests around readlinkat() and mkdirat().

#Refs 1188

Signed-off-by: Waldemar Kozaczuk 
---
 fs/vfs/main.cc   | 216 ---
 tests/tst-readdir.cc |   9 ++
 tests/tst-symlink.cc |  12 ++-
 3 files changed, 81 insertions(+), 156 deletions(-)

diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
index ca357cc8..1b7eb588 100644
--- a/fs/vfs/main.cc
+++ b/fs/vfs/main.cc
@@ -160,21 +160,8 @@ int open(const char *pathname, int flags, ...)
 
 LFS64(open);
 
-OSV_LIBC_API
-int openat(int dirfd, const char *pathname, int flags, ...)
+static int vfs_fun_at(int dirfd, const char *pathname, std::function fun)
 {
-mode_t mode = 0;
-if (flags & O_CREAT) {
-va_list ap;
-va_start(ap, flags);
-mode = apply_umask(va_arg(ap, mode_t));
-va_end(ap);
-}
-
-if (pathname[0] == '/' || dirfd == AT_FDCWD) {
-return open(pathname, flags, mode);
-}
-
 struct file *fp;
 int error = fget(dirfd, );
 if (error) {
@@ -191,16 +178,48 @@ int openat(int dirfd, const char *pathname, int flags, 
...)
 /* build absolute path */
 strlcpy(p, fp->f_dentry->d_mount->m_path, PATH_MAX);
 strlcat(p, fp->f_dentry->d_path, PATH_MAX);
-strlcat(p, "/", PATH_MAX);
-strlcat(p, pathname, PATH_MAX);
+if (pathname) {
+strlcat(p, "/", PATH_MAX);
+strlcat(p, pathname, PATH_MAX);
+}
 
-error = open(p, flags, mode);
+error = fun(p);
 
 vn_unlock(vp);
 fdrop(fp);
 
 return error;
 }
+
+static int vfs_fun_at2(int dirfd, const char *pathname, 
std::function fun)
+{
+if (!pathname) {
+errno = EINVAL;
+return -1;
+}
+
+if (pathname[0] == '/' || dirfd == AT_FDCWD) {
+return fun(pathname);
+}
+
+return vfs_fun_at(dirfd, pathname, fun);
+}
+
+OSV_LIBC_API
+int openat(int dirfd, const char *pathname, int flags, ...)
+{
+mode_t mode = 0;
+if (flags & O_CREAT) {
+va_list ap;
+va_start(ap, flags);
+mode = apply_umask(va_arg(ap, mode_t));
+va_end(ap);
+}
+
+return vfs_fun_at2(dirfd, pathname, [flags, mode](const char *path) {
+return open(path, flags, mode);
+});
+}
 LFS64(openat);
 
 // open() has an optional third argument, "mode", which is only needed in
@@ -602,6 +621,11 @@ extern "C" OSV_LIBC_API
 int __fxstatat(int ver, int dirfd, const char *pathname, struct stat *st,
 int flags)
 {
+if (!pathname) {
+errno = EINVAL;
+return -1;
+}
+
 if (pathname[0] == '/' || dirfd == AT_FDCWD) {
 return stat(pathname, st);
 }
@@ -611,35 +635,14 @@ int __fxstatat(int ver, int dirfd, const char *pathname, 
struct stat *st,
 return fstat(dirfd, st);
 }
 
-struct file *fp;
-int error = fget(dirfd, );
-if (error) {
-errno = error;
-return -1;
-}
-
-struct vnode *vp = fp->f_dentry->d_vnode;
-vn_lock(vp);
-
-std::unique_ptr up (new char[PATH_MAX]);
-char *p = up.get();
-/* build absolute path */
-strlcpy(p, fp->f_dentry->d_mount->m_path, PATH_MAX);
-strlcat(p, fp->f_dentry->d_path, PATH_MAX);
-strlcat(p, "/", PATH_MAX);
-strlcat(p, pathname, PATH_MAX);
-
-if (flags & AT_SYMLINK_NOFOLLOW) {
-error = lstat(p, st);
-}
-else {
-error = stat(p, st);
-}
-
-vn_unlock(vp);
-fdrop(fp);
-
-return error;
+return vfs_fun_at(dirfd, pathname, [flags,st](const char *absolute_path) {
+if (flags & AT_SYMLINK_NOFOLLOW) {
+return lstat(absolute_path, st);
+}
+else {
+return stat(absolute_path, st);
+}
+});
 }
 
 LFS64(__fxstatat);
@@ -870,37 +873,9 @@ int mkdirat(int dirfd, const char *pathname, mode_t mode)
 {
 mode = apply_umask(mode);
 
-   if (pathname[0] == '/' || dirfd == AT_FDCWD) {
-// Supplied path is either absolute or relative to cwd
-return mkdir(pathname, mode);
-}
-
-// Supplied path is relative to folder specified by dirfd
-struct file *fp;
-int error = fget(dirfd, );
-if (error) {
-errno = error;
-return -1;
-}
-
-struct vnode *vp =