From 7d288aafb25f44a63cf1e4c3e262b4906bc12b21 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 21 Feb 2019 13:26:40 +0100 Subject: [PATCH] Kernel: Add link() syscall to create hard links. This accidentally grew into a little bit of VFS cleanup as well. Also add a simple /bin/ln implementation to exercise it. --- Kernel/Ext2FileSystem.cpp | 13 +-- Kernel/ProcFS.cpp | 10 +-- Kernel/Process.cpp | 20 ++++- Kernel/Process.h | 1 + Kernel/Syscall.cpp | 2 + Kernel/Syscall.h | 1 + Kernel/VirtualFileSystem.cpp | 152 ++++++++++++++++++----------------- Kernel/VirtualFileSystem.h | 2 + Kernel/sync.sh | 1 + LibC/unistd.cpp | 5 +- Userland/.gitignore | 1 + Userland/Makefile | 5 ++ Userland/ln.cpp | 18 +++++ 13 files changed, 138 insertions(+), 93 deletions(-) create mode 100644 Userland/ln.cpp diff --git a/Kernel/Ext2FileSystem.cpp b/Kernel/Ext2FileSystem.cpp index 1ac6f1acfab..4ef19c48bc2 100644 --- a/Kernel/Ext2FileSystem.cpp +++ b/Kernel/Ext2FileSystem.cpp @@ -637,12 +637,14 @@ bool Ext2FSInode::add_child(InodeIdentifier child_id, const String& name, byte f return false; } + auto child_inode = fs().get_inode(child_id); + if (child_inode) + child_inode->increment_link_count(); + entries.append({ name.characters(), name.length(), child_id, file_type }); bool success = fs().write_directory_inode(index(), move(entries)); - if (success) { - LOCKER(m_lock); + if (success) m_lookup_cache.set(name, child_id.index()); - } return success; } @@ -672,8 +674,8 @@ bool Ext2FSInode::remove_child(const String& name, int& error) Vector entries; traverse_as_directory([&] (auto& entry) { - if (entry.inode != child_id) - entries.append(entry); + if (strcmp(entry.name, name.characters()) != 0) + entries.append(entry); return true; }); @@ -1336,6 +1338,7 @@ int Ext2FSInode::decrement_link_count() void Ext2FS::uncache_inode(InodeIndex index) { LOCKER(m_lock); + m_inode_cache.remove(index); } size_t Ext2FSInode::directory_entry_count() const diff --git a/Kernel/ProcFS.cpp b/Kernel/ProcFS.cpp index 7cdd3c9f8f2..93258a24203 100644 --- a/Kernel/ProcFS.cpp +++ b/Kernel/ProcFS.cpp @@ -1038,19 +1038,13 @@ ssize_t ProcFSInode::write_bytes(off_t offset, size_t size, const byte* buffer, bool ProcFSInode::add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) { - (void) child_id; - (void) name; - (void) file_type; - (void) error; - ASSERT_NOT_REACHED(); + error = -EPERM; return false; } bool ProcFSInode::remove_child(const String& name, int& error) { - (void) name; - (void) error; - ASSERT_NOT_REACHED(); + error = -EPERM; return false; } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index f0ae095cea6..607a1f56014 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -1143,9 +1143,11 @@ int Process::sys$utime(const char* pathname, const utimbuf* buf) mtime = now; atime = now; } - inode.set_atime(atime); - inode.set_mtime(mtime); - return 0; + error = inode.set_atime(atime); + if (error) + return error; + error = inode.set_mtime(mtime); + return error; } int Process::sys$access(const char* pathname, int mode) @@ -2099,6 +2101,18 @@ Inode* Process::cwd_inode() return m_cwd.ptr(); } +int Process::sys$link(const char* old_path, const char* new_path) +{ + if (!validate_read_str(old_path)) + return -EFAULT; + if (!validate_read_str(new_path)) + return -EFAULT; + int error; + if (!VFS::the().link(String(old_path), String(new_path), *cwd_inode(), error)) + return error; + return 0; +} + int Process::sys$unlink(const char* pathname) { if (!validate_read_str(pathname)) diff --git a/Kernel/Process.h b/Kernel/Process.h index b44fda0dce6..f77996105c0 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -210,6 +210,7 @@ public: int sys$mkdir(const char* pathname, mode_t mode); clock_t sys$times(tms*); int sys$utime(const char* pathname, const struct utimbuf*); + int sys$link(const char* old_path, const char* new_path); int sys$unlink(const char* pathname); int sys$rmdir(const char* pathname); int sys$read_tsc(dword* lsw, dword* msw); diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp index 65f6c12a39c..f47ae4d644d 100644 --- a/Kernel/Syscall.cpp +++ b/Kernel/Syscall.cpp @@ -193,6 +193,8 @@ static dword handle(RegisterDump& regs, dword function, dword arg1, dword arg2, return current->sys$utime((const char*)arg1, (const utimbuf*)arg2); case Syscall::SC_sync: return sync(); + case Syscall::SC_link: + return current->sys$link((const char*)arg1, (const char*)arg2); case Syscall::SC_unlink: return current->sys$unlink((const char*)arg1); case Syscall::SC_read_tsc: diff --git a/Kernel/Syscall.h b/Kernel/Syscall.h index d250e1c0957..9eaacab3584 100644 --- a/Kernel/Syscall.h +++ b/Kernel/Syscall.h @@ -82,6 +82,7 @@ __ENUMERATE_SYSCALL(create_shared_buffer) \ __ENUMERATE_SYSCALL(get_shared_buffer) \ __ENUMERATE_SYSCALL(release_shared_buffer) \ + __ENUMERATE_SYSCALL(link) \ #ifdef SERENITY diff --git a/Kernel/VirtualFileSystem.cpp b/Kernel/VirtualFileSystem.cpp index 55f2c41c9d2..64876669f5b 100644 --- a/Kernel/VirtualFileSystem.cpp +++ b/Kernel/VirtualFileSystem.cpp @@ -168,28 +168,22 @@ RetainPtr VFS::create(const String& path, int& error, int option mode |= 0100000; } - // FIXME: This won't work nicely across mount boundaries. - FileSystemPath p(path); - if (!p.is_valid()) { - error = -EINVAL; - return nullptr; - } - - InodeIdentifier parent_dir; - auto existing_file = resolve_path(path, base.identifier(), error, 0, &parent_dir); - if (existing_file.is_valid()) { + RetainPtr parent_inode; + auto existing_file = resolve_path_to_inode(path, base, error, &parent_inode); + if (existing_file) { error = -EEXIST; return nullptr; } - if (!parent_dir.is_valid()) { + if (!parent_inode) { error = -ENOENT; return nullptr; } if (error != -ENOENT) { return nullptr; } - dbgprintf("VFS::create_file: '%s' in %u:%u\n", p.basename().characters(), parent_dir.fsid(), parent_dir.index()); - auto new_file = parent_dir.fs()->create_inode(parent_dir, p.basename(), mode, 0, error); + FileSystemPath p(path); + dbgprintf("VFS::create_file: '%s' in %u:%u\n", p.basename().characters(), parent_inode->fsid(), parent_inode->index()); + auto new_file = parent_inode->fs().create_inode(parent_inode->identifier(), p.basename(), mode, 0, error); if (!new_file) return nullptr; @@ -200,28 +194,23 @@ RetainPtr VFS::create(const String& path, int& error, int option bool VFS::mkdir(const String& path, mode_t mode, Inode& base, int& error) { error = -EWHYTHO; - // FIXME: This won't work nicely across mount boundaries. - FileSystemPath p(path); - if (!p.is_valid()) { - error = -EINVAL; - return false; - } - InodeIdentifier parent_dir; - auto existing_dir = resolve_path(path, base.identifier(), error, 0, &parent_dir); - if (existing_dir.is_valid()) { + RetainPtr parent_inode; + auto existing_dir = resolve_path_to_inode(path, base, error, &parent_inode); + if (existing_dir) { error = -EEXIST; return false; } - if (!parent_dir.is_valid()) { + if (!parent_inode) { error = -ENOENT; return false; } if (error != -ENOENT) { return false; } - dbgprintf("VFS::mkdir: '%s' in %u:%u\n", p.basename().characters(), parent_dir.fsid(), parent_dir.index()); - auto new_dir = parent_dir.fs()->create_directory(parent_dir, p.basename(), mode, error); + FileSystemPath p(path); + dbgprintf("VFS::mkdir: '%s' in %u:%u\n", p.basename().characters(), parent_inode->fsid(), parent_inode->index()); + auto new_dir = parent_inode->fs().create_directory(parent_inode->identifier(), p.basename(), mode, error); if (new_dir) { error = 0; return true; @@ -232,60 +221,85 @@ bool VFS::mkdir(const String& path, mode_t mode, Inode& base, int& error) bool VFS::chmod(const String& path, mode_t mode, Inode& base, int& error) { error = -EWHYTHO; - // FIXME: This won't work nicely across mount boundaries. - FileSystemPath p(path); - if (!p.is_valid()) { - error = -EINVAL; + auto inode = resolve_path_to_inode(path, base, error); + if (!inode) return false; - } - - InodeIdentifier parent_dir; - auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_dir); - if (!inode_id.is_valid()) { - error = -ENOENT; - return false; - } - - auto inode = get_inode(inode_id); // FIXME: Permission checks. // Only change the permission bits. mode = (inode->mode() & ~04777) | (mode & 04777); - kprintf("VFS::chmod(): %u:%u mode %o\n", inode_id.fsid(), inode_id.index(), mode); + kprintf("VFS::chmod(): %u:%u mode %o\n", inode->fsid(), inode->index(), mode); if (!inode->chmod(mode, error)) return false; error = 0; return true; } +RetainPtr VFS::resolve_path_to_inode(const String& path, Inode& base, int& error, RetainPtr* parent_inode) +{ + // FIXME: This won't work nicely across mount boundaries. + FileSystemPath p(path); + if (!p.is_valid()) { + error = -EINVAL; + return nullptr; + } + InodeIdentifier parent_id; + auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_id); + if (parent_inode && parent_id.is_valid()) + *parent_inode = get_inode(parent_id); + if (!inode_id.is_valid()) { + error = -ENOENT; + return nullptr; + } + return get_inode(inode_id); +} + +bool VFS::link(const String& old_path, const String& new_path, Inode& base, int& error) +{ + auto old_inode = resolve_path_to_inode(old_path, base, error); + if (!old_inode) + return false; + + RetainPtr parent_inode; + auto new_inode = resolve_path_to_inode(new_path, base, error, &parent_inode); + if (new_inode) { + error = -EEXIST; + return false; + } + if (!parent_inode) { + error = -ENOENT; + return false; + } + if (parent_inode->fsid() != old_inode->fsid()) { + error = -EXDEV; + return false; + } + if (parent_inode->fs().is_readonly()) { + error = -EROFS; + return false; + } + + if (!parent_inode->add_child(old_inode->identifier(), FileSystemPath(old_path).basename(), 0, error)) + return false; + error = 0; + return true; +} + bool VFS::unlink(const String& path, Inode& base, int& error) { - error = -EWHYTHO; - // FIXME: This won't work nicely across mount boundaries. - FileSystemPath p(path); - if (!p.is_valid()) { - error = -EINVAL; + RetainPtr parent_inode; + auto inode = resolve_path_to_inode(path, base, error, &parent_inode); + if (!inode) return false; - } - InodeIdentifier parent_dir; - auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_dir); - if (!inode_id.is_valid()) { - error = -ENOENT; - return false; - } - - auto inode = get_inode(inode_id); if (inode->is_directory()) { error = -EISDIR; return false; } - auto parent_inode = get_inode(parent_dir); - // FIXME: The reverse_lookup here can definitely be avoided. - if (!parent_inode->remove_child(parent_inode->reverse_lookup(inode_id), error)) + if (!parent_inode->remove_child(FileSystemPath(path).basename(), error)) return false; error = 0; @@ -295,21 +309,13 @@ bool VFS::unlink(const String& path, Inode& base, int& error) bool VFS::rmdir(const String& path, Inode& base, int& error) { error = -EWHYTHO; - // FIXME: This won't work nicely across mount boundaries. - FileSystemPath p(path); - if (!p.is_valid()) { - error = -EINVAL; - return false; - } - InodeIdentifier parent_dir; - auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_dir); - if (!inode_id.is_valid()) { - error = -ENOENT; + RetainPtr parent_inode; + auto inode = resolve_path_to_inode(path, base, error, &parent_inode); + if (!inode) return false; - } - if (inode_id.fs()->is_readonly()) { + if (inode->fs().is_readonly()) { error = -EROFS; return false; } @@ -317,7 +323,6 @@ bool VFS::rmdir(const String& path, Inode& base, int& error) // FIXME: We should return EINVAL if the last component of the path is "." // FIXME: We should return ENOTEMPTY if the last component of the path is ".." - auto inode = get_inode(inode_id); if (!inode->is_directory()) { error = -ENOTDIR; return false; @@ -328,10 +333,7 @@ bool VFS::rmdir(const String& path, Inode& base, int& error) return false; } - auto parent_inode = get_inode(parent_dir); - ASSERT(parent_inode); - - dbgprintf("VFS::rmdir: Removing inode %u:%u from parent %u:%u\n", inode_id.fsid(), inode_id.index(), parent_dir.fsid(), parent_dir.index()); + dbgprintf("VFS::rmdir: Removing inode %u:%u from parent %u:%u\n", inode->fsid(), inode->index(), parent_inode->fsid(), parent_inode->index()); // To do: // - Remove '.' in target (--child.link_count) @@ -344,7 +346,7 @@ bool VFS::rmdir(const String& path, Inode& base, int& error) return false; // FIXME: The reverse_lookup here can definitely be avoided. - if (!parent_inode->remove_child(parent_inode->reverse_lookup(inode_id), error)) + if (!parent_inode->remove_child(parent_inode->reverse_lookup(inode->identifier()), error)) return false; error = 0; diff --git a/Kernel/VirtualFileSystem.h b/Kernel/VirtualFileSystem.h index f5b57c98a0e..ca822ce5fa8 100644 --- a/Kernel/VirtualFileSystem.h +++ b/Kernel/VirtualFileSystem.h @@ -66,6 +66,7 @@ public: RetainPtr open(const String& path, int& error, int options, mode_t mode, Inode& base); RetainPtr create(const String& path, int& error, int options, mode_t mode, Inode& base); bool mkdir(const String& path, mode_t mode, Inode& base, int& error); + bool link(const String& old_path, const String& new_path, Inode& base, int& error); bool unlink(const String& path, Inode& base, int& error); bool rmdir(const String& path, Inode& base, int& error); bool chmod(const String& path, mode_t, Inode& base, int& error); @@ -96,6 +97,7 @@ private: void traverse_directory_inode(Inode&, Function); InodeIdentifier resolve_path(const String& path, InodeIdentifier base, int& error, int options = 0, InodeIdentifier* parent_id = nullptr); + RetainPtr resolve_path_to_inode(const String& path, Inode& base, int& error, RetainPtr* parent_id = nullptr); InodeIdentifier resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode, int& error); Mount* find_mount_for_host(InodeIdentifier); diff --git a/Kernel/sync.sh b/Kernel/sync.sh index 6698da82394..69e7890e795 100755 --- a/Kernel/sync.sh +++ b/Kernel/sync.sh @@ -63,6 +63,7 @@ cp -v ../Userland/pape mnt/bin/pape cp -v ../Userland/dmesg mnt/bin/dmesg cp -v ../Userland/chmod mnt/bin/chmod cp -v ../Userland/top mnt/bin/top +cp -v ../Userland/ln mnt/bin/ln cp -v ../Applications/Terminal/Terminal mnt/bin/Terminal cp -v ../Applications/FontEditor/FontEditor mnt/bin/FontEditor cp -v ../Applications/Launcher/Launcher mnt/bin/Launcher diff --git a/LibC/unistd.cpp b/LibC/unistd.cpp index d6ec950f7da..6ae189210cc 100644 --- a/LibC/unistd.cpp +++ b/LibC/unistd.cpp @@ -229,9 +229,10 @@ off_t lseek(int fd, off_t offset, int whence) __RETURN_WITH_ERRNO(rc, rc, -1); } -int link(const char*, const char*) +int link(const char* old_path, const char* new_path) { - assert(false); + int rc = syscall(SC_link, old_path, new_path); + __RETURN_WITH_ERRNO(rc, rc, -1); } int unlink(const char* pathname) diff --git a/Userland/.gitignore b/Userland/.gitignore index a3644d8cdcd..4c4f51262ad 100644 --- a/Userland/.gitignore +++ b/Userland/.gitignore @@ -31,3 +31,4 @@ dmesg top chmod pape +ln diff --git a/Userland/Makefile b/Userland/Makefile index 6868ce21a43..4759c51e24c 100644 --- a/Userland/Makefile +++ b/Userland/Makefile @@ -27,6 +27,7 @@ OBJS = \ dmesg.o \ chmod.o \ top.o \ + ln.o \ rm.o APPS = \ @@ -59,6 +60,7 @@ APPS = \ dmesg \ chmod \ top \ + ln \ rm ARCH_FLAGS = @@ -169,6 +171,9 @@ chmod: chmod.o top: top.o $(LD) -o $@ $(LDFLAGS) $< ../LibC/LibC.a +ln: ln.o + $(LD) -o $@ $(LDFLAGS) $< ../LibC/LibC.a + .cpp.o: @echo "CXX $<"; $(CXX) $(CXXFLAGS) -o $@ -c $< diff --git a/Userland/ln.cpp b/Userland/ln.cpp new file mode 100644 index 00000000000..d591be2aff3 --- /dev/null +++ b/Userland/ln.cpp @@ -0,0 +1,18 @@ +#include +#include +#include + +int main(int argc, char** argv) +{ + if (argc != 3) { + fprintf(stderr, "usage: ln \n"); + return 1; + } + int rc = link(argv[1], argv[2]); + if (rc < 0) { + perror("link"); + return 1; + } + return 0; +} +