From: Waldemar Kozaczuk <[email protected]> Committer: Waldemar Kozaczuk <[email protected]> Branch: master
vfs: refactor the *at() functions 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 <[email protected]> --- diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc --- 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<int(const char *)> 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, &fp); 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<int(const char *)> 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, &fp); - if (error) { - errno = error; - return -1; - } - - struct vnode *vp = fp->f_dentry->d_vnode; - vn_lock(vp); - - std::unique_ptr<char []> 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, &fp); - if (error) { - errno = error; - return -1; - } - - struct vnode *vp = fp->f_dentry->d_vnode; - vn_lock(vp); - - std::unique_ptr<char []> 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); - - error = mkdir(p, mode); - - vn_unlock(vp); - fdrop(fp); - - return error; + return vfs_fun_at2(dirfd, pathname, [mode](const char *path) { + return mkdir(path, mode); + }); } TRACEPOINT(trace_vfs_rmdir, "\"%s\"", const char*); @@ -1717,35 +1692,9 @@ int faccessat(int dirfd, const char *pathname, int mode, int flags) UNIMPLEMENTED("faccessat() with AT_SYMLINK_NOFOLLOW"); } - if (pathname[0] == '/' || dirfd == AT_FDCWD) { - return access(pathname, mode); - } - - struct file *fp; - int error = fget(dirfd, &fp); - if (error) { - errno = error; - return -1; - } - - struct vnode *vp = fp->f_dentry->d_vnode; - vn_lock(vp); - - std::unique_ptr<char []> 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); - - error = access(p, mode); - - vn_unlock(vp); - fdrop(fp); - - return error; + return vfs_fun_at2(dirfd, pathname, [mode](const char *path) { + return access(path, mode); + }); } extern "C" OSV_LIBC_API @@ -1928,35 +1877,9 @@ ssize_t readlink(const char *pathname, char *buf, size_t bufsize) OSV_LIBC_API ssize_t readlinkat(int dirfd, const char *pathname, char *buf, size_t bufsize) { - if (pathname[0] == '/' || dirfd == AT_FDCWD) { - return readlink(pathname, buf, bufsize); - } - - struct file *fp; - int error = fget(dirfd, &fp); - if (error) { - errno = error; - return -1; - } - - struct vnode *vp = fp->f_dentry->d_vnode; - vn_lock(vp); - - std::unique_ptr<char []> 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); - - error = readlink(p, buf, bufsize); - - vn_unlock(vp); - fdrop(fp); - - return error; + return vfs_fun_at2(dirfd, pathname, [buf, bufsize](const char *path) { + return readlink(path, buf, bufsize); + }); } TRACEPOINT(trace_vfs_fallocate, "%d %d 0x%x 0x%x", int, int, loff_t, loff_t); @@ -2004,9 +1927,7 @@ OSV_LIBC_API int futimesat(int dirfd, const char *pathname, const struct timeval times[2]) { struct stat st; - struct file *fp; int error; - char *absolute_path; if ((pathname && pathname[0] == '/') || dirfd == AT_FDCWD) return utimes(pathname, times); @@ -2026,24 +1947,9 @@ int futimesat(int dirfd, const char *pathname, const struct timeval times[2]) } } - error = fget(dirfd, &fp); - if (error) - goto out_errno; - - /* build absolute path */ - absolute_path = (char*)malloc(PATH_MAX); - strlcpy(absolute_path, fp->f_dentry->d_mount->m_path, PATH_MAX); - strlcat(absolute_path, fp->f_dentry->d_path, PATH_MAX); - - if (pathname) { - strlcat(absolute_path, "/", PATH_MAX); - strlcat(absolute_path, pathname, PATH_MAX); - } - - error = utimes(absolute_path, times); - free(absolute_path); - - fdrop(fp); + error = vfs_fun_at(dirfd, pathname, [times](const char *absolute_path) { + return utimes(absolute_path, times); + }); if (error) goto out_errno; diff --git a/tests/tst-readdir.cc b/tests/tst-readdir.cc --- a/tests/tst-readdir.cc +++ b/tests/tst-readdir.cc @@ -216,6 +216,15 @@ int main(int argc, char **argv) report(unlink("/tmp/tst-readdir/d")==0, "unlink"); report(rmdir("/tmp/tst-readdir")==0, "rmdir"); report(rmdir("/tmp/tst-readdir-empty")==0, "rmdir empty"); + + auto tmp_dir = opendir("/tmp"); + report(tmp_dir,"opendir /tmp"); + report(mkdirat(dirfd(tmp_dir), "tst-mkdirat", 0777)==0, "mkdirat"); + auto mk_dir = opendir("/tmp/tst-mkdirat"); + report(mk_dir,"opendir /tmp/tst-mkdirat"); + report(rmdir("/tmp/tst-mkdirat")==0, "rmdir empty"); + report(closedir(mk_dir)==0, "closedir /tmp/tst-mkdirat"); + report(closedir(tmp_dir)==0, "closedir /tmp"); #endif printf("SUMMARY: %d tests, %d failures\n", tests, fails); diff --git a/tests/tst-symlink.cc b/tests/tst-symlink.cc --- a/tests/tst-symlink.cc +++ b/tests/tst-symlink.cc @@ -262,7 +262,6 @@ int main(int argc, char **argv) report(symlink(N1, path) == 0, "symlink"); report(unlink(path) == 0, "unlink"); - printf("-->Smok\n"); fill_buf(path, 257); rc = symlink(N1, path); error = errno; @@ -366,12 +365,23 @@ 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; + report(strcmp(path, D1) == 0, "readlinkat path"); rc = readlink(D3, path, sizeof(path)); report(rc >= 0, "readlink"); path[rc] = 0; report(strcmp(path, D1) == 0, "readlink path"); report(rename(D1, D4) == 0, "rename(d1, d4)"); + rc = readlinkat(dirfd(test_dir), D3, path, sizeof(path)); + 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; -- 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/000000000000e0f77e05dfffc522%40google.com.
