From: Waldemar Kozaczuk <jwkozac...@gmail.com>
Committer: Waldemar Kozaczuk <jwkozac...@gmail.com>
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 <jwkozac...@gmail.com>
---
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 osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/000000000000e0f77e05dfffc522%40google.com.
