On Wed, May 18, 2022 at 6:26 AM Waldemar Kozaczuk <jwkozac...@gmail.com>
wrote:
> 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 <jwkozac...@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);
> + }
> + });
>
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.
> }
>
> 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
>
> --
> 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/20220518032614.76774-1-jwkozaczuk%40gmail.com
> .
>
--
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/CANEVyjv2b9V_KH%3Dk2264-4%3D8815gUCKMP6eMHFxRwN%3D1AKHDmw%40mail.gmail.com.
