[PATCH] unlinkat: fill the gaps in the implementation

[Waldemar Kozaczuk]

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.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
fs/vfs/main.cc | 18 ++++++++++++++----
tests/tst-remove.cc | 18 ++++++++++++++++--
2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
index a72042b2..4f0ce463 100644
--- a/fs/vfs/main.cc
+++ b/fs/vfs/main.cc
@@ -1154,11 +1154,21 @@ 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");
+ if (pathname[0] == '/' || dirfd == AT_FDCWD) {
+ if (flags & AT_REMOVEDIR) {
+ return rmdir(pathname);
+ } else {
+ return unlink(pathname);
+ }
}
- return unlink(pathname);
+
+ return vfs_fun_at(dirfd, pathname, [flags](const char *absolute_path) {
+ if (flags & AT_REMOVEDIR) {
+ return rmdir(absolute_path);
+ } else {
+ return unlink(absolute_path);
+ }
+ });
}

TRACEPOINT(trace_vfs_stat, "\"%s\" %p", const char*, struct stat*);
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

[Nadav Har'El]

In my review of my other patch, I suggested that the AT_FDCWD thing should be part of vfs_fun_at().

After all you can clearly see that you have exactly the same lambda function twice in this code, it's

almost begging to have just one copy of this lambda, and vfs_fun_at will just call it in one of two ways.

 

--
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+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20220518032614.76774-1-jwkozaczuk%40gmail.com.

[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 <jwkoz...@gmail.com>
---

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);

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

unlinkat: fill the gaps in the implementation

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 <jwkoz...@gmail.com>

---

diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc

--- 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

--- 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