diff --git a/Kernel/Ext2FileSystem.cpp b/Kernel/Ext2FileSystem.cpp index 8880b5f7d0..ef0e24ef92 100644 --- a/Kernel/Ext2FileSystem.cpp +++ b/Kernel/Ext2FileSystem.cpp @@ -649,7 +649,7 @@ bool Ext2FSInode::add_child(InodeIdentifier child_id, const String& name, byte f return success; } -bool Ext2FSInode::remove_child(const String& name, int& error) +KResult Ext2FSInode::remove_child(const String& name) { LOCKER(m_lock); #ifdef EXT2_DEBUG @@ -658,15 +658,11 @@ bool Ext2FSInode::remove_child(const String& name, int& error) ASSERT(is_directory()); unsigned child_inode_index; - { - LOCKER(m_lock); - auto it = m_lookup_cache.find(name); - if (it == m_lookup_cache.end()) { - error = -ENOENT; - return false; - } - child_inode_index = (*it).value; - } + auto it = m_lookup_cache.find(name); + if (it == m_lookup_cache.end()) + return KResult(-ENOENT); + child_inode_index = (*it).value; + InodeIdentifier child_id { fsid(), child_inode_index }; //#ifdef EXT2_DEBUG @@ -683,18 +679,14 @@ bool Ext2FSInode::remove_child(const String& name, int& error) bool success = fs().write_directory_inode(index(), move(entries)); if (!success) { // FIXME: Plumb error from write_directory_inode(). - error = -EIO; - return false; + return KResult(-EIO); } - { - LOCKER(m_lock); - m_lookup_cache.remove(name); - } + m_lookup_cache.remove(name); auto child_inode = fs().get_inode(child_id); child_inode->decrement_link_count(); - return success; + return KSuccess; } bool Ext2FS::write_directory_inode(unsigned directoryInode, Vector&& entries) diff --git a/Kernel/Ext2FileSystem.h b/Kernel/Ext2FileSystem.h index eddabe946a..ca93ddca60 100644 --- a/Kernel/Ext2FileSystem.h +++ b/Kernel/Ext2FileSystem.h @@ -33,7 +33,7 @@ private: virtual void flush_metadata() override; virtual ssize_t write_bytes(off_t, ssize_t, const byte* data, FileDescriptor*) override; virtual bool add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) override; - virtual bool remove_child(const String& name, int& error) override; + virtual KResult remove_child(const String& name) override; virtual RetainPtr parent() const override; virtual int set_atime(time_t) override; virtual int set_ctime(time_t) override; diff --git a/Kernel/FileSystem.h b/Kernel/FileSystem.h index e11c0ad6a3..1fdfb56c08 100644 --- a/Kernel/FileSystem.h +++ b/Kernel/FileSystem.h @@ -94,7 +94,7 @@ public: virtual String reverse_lookup(InodeIdentifier) = 0; virtual ssize_t write_bytes(off_t, ssize_t, const byte* data, FileDescriptor*) = 0; virtual bool add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) = 0; - virtual bool remove_child(const String& name, int& error) = 0; + virtual KResult remove_child(const String& name) = 0; virtual RetainPtr parent() const = 0; virtual size_t directory_entry_count() const = 0; virtual KResult chmod(mode_t) = 0; diff --git a/Kernel/KResult.h b/Kernel/KResult.h index aa4e0bb3e3..a954633973 100644 --- a/Kernel/KResult.h +++ b/Kernel/KResult.h @@ -11,6 +11,9 @@ public: KResult(KSuccessTag) : m_error(0) { } operator int() const { return m_error; } + bool is_success() const { return m_error == ESUCCESS; } + bool is_error() const { return !is_success(); } + private: template friend class KResultOr; KResult() { } diff --git a/Kernel/ProcFS.cpp b/Kernel/ProcFS.cpp index a46036845e..72c4d0d0e1 100644 --- a/Kernel/ProcFS.cpp +++ b/Kernel/ProcFS.cpp @@ -1056,11 +1056,10 @@ bool ProcFSInode::add_child(InodeIdentifier child_id, const String& name, byte f return false; } -bool ProcFSInode::remove_child(const String& name, int& error) +KResult ProcFSInode::remove_child(const String& name) { (void)name; - error = -EPERM; - return false; + return KResult(-EPERM); } ProcFSInodeCustomData::~ProcFSInodeCustomData() diff --git a/Kernel/ProcFS.h b/Kernel/ProcFS.h index 89b1635e65..563a1ef745 100644 --- a/Kernel/ProcFS.h +++ b/Kernel/ProcFS.h @@ -84,7 +84,7 @@ private: virtual void flush_metadata() override; virtual ssize_t write_bytes(off_t, ssize_t, const byte* buffer, FileDescriptor*) override; virtual bool add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) override; - virtual bool remove_child(const String& name, int& error) override; + virtual KResult remove_child(const String& name) override; virtual RetainPtr parent() const override; virtual size_t directory_entry_count() const override; virtual KResult chmod(mode_t) override; diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index e2e05805f6..d8c8ec9dd5 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -2129,20 +2129,14 @@ int Process::sys$unlink(const char* pathname) { if (!validate_read_str(pathname)) return -EFAULT; - int error; - if (!VFS::the().unlink(String(pathname), cwd_inode(), error)) - return error; - return 0; + return VFS::the().unlink(String(pathname), cwd_inode()); } int Process::sys$rmdir(const char* pathname) { if (!validate_read_str(pathname)) return -EFAULT; - int error; - if (!VFS::the().rmdir(String(pathname), cwd_inode(), error)) - return error; - return 0; + return VFS::the().rmdir(String(pathname), cwd_inode()); } int Process::sys$read_tsc(dword* lsw, dword* msw) diff --git a/Kernel/SyntheticFileSystem.cpp b/Kernel/SyntheticFileSystem.cpp index 999615d66d..670aa2fcb2 100644 --- a/Kernel/SyntheticFileSystem.cpp +++ b/Kernel/SyntheticFileSystem.cpp @@ -290,12 +290,10 @@ bool SynthFSInode::add_child(InodeIdentifier child_id, const String& name, byte return false; } -bool SynthFSInode::remove_child(const String& name, int& error) +KResult SynthFSInode::remove_child(const String& name) { - (void) name; - (void) error; + (void)name; ASSERT_NOT_REACHED(); - return false; } SynthFSInodeCustomData::~SynthFSInodeCustomData() diff --git a/Kernel/SyntheticFileSystem.h b/Kernel/SyntheticFileSystem.h index 128584526c..d991076af9 100644 --- a/Kernel/SyntheticFileSystem.h +++ b/Kernel/SyntheticFileSystem.h @@ -64,7 +64,7 @@ private: virtual void flush_metadata() override; virtual ssize_t write_bytes(off_t, ssize_t, const byte* buffer, FileDescriptor*) override; virtual bool add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) override; - virtual bool remove_child(const String& name, int& error) override; + virtual KResult remove_child(const String& name) override; virtual RetainPtr parent() const override; virtual size_t directory_entry_count() const override; virtual KResult chmod(mode_t) override; diff --git a/Kernel/VirtualFileSystem.cpp b/Kernel/VirtualFileSystem.cpp index 45acec8e89..7971c5ad3b 100644 --- a/Kernel/VirtualFileSystem.cpp +++ b/Kernel/VirtualFileSystem.cpp @@ -410,80 +410,56 @@ bool VFS::link(const String& old_path, const String& new_path, Inode& base, int& return true; } -bool VFS::unlink(const String& path, Inode& base, int& error) +KResult VFS::unlink(const String& path, Inode& base) { RetainPtr parent_inode; - auto inode = resolve_path_to_inode(path, base, error, &parent_inode); - if (!inode) - return false; + auto inode_or_error = resolve_path_to_inode(path, base, &parent_inode); + if (inode_or_error.is_error()) + return inode_or_error.error(); + auto inode = inode_or_error.value(); - if (inode->is_directory()) { - error = -EISDIR; - return false; - } + if (inode->is_directory()) + return KResult(-EISDIR); - if (!parent_inode->metadata().may_write(*current)) { - error = -EACCES; - return false; - } + if (!parent_inode->metadata().may_write(*current)) + return KResult(-EACCES); - if (!parent_inode->remove_child(FileSystemPath(path).basename(), error)) - return false; - - error = 0; - return true; + return parent_inode->remove_child(FileSystemPath(path).basename()); } -bool VFS::rmdir(const String& path, Inode& base, int& error) +KResult VFS::rmdir(const String& path, Inode& base) { - error = -EWHYTHO; - RetainPtr parent_inode; - auto inode = resolve_path_to_inode(path, base, error, &parent_inode); - if (!inode) - return false; + auto inode_or_error = resolve_path_to_inode(path, base, &parent_inode); + if (inode_or_error.is_error()) + return KResult(inode_or_error.error()); - if (inode->fs().is_readonly()) { - error = -EROFS; - return false; - } + auto inode = inode_or_error.value(); + if (inode->fs().is_readonly()) + return KResult(-EROFS); // 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 ".." - if (!inode->is_directory()) { - error = -ENOTDIR; - return false; - } + if (!inode->is_directory()) + return KResult(-ENOTDIR); - if (!parent_inode->metadata().may_write(*current)) { - error = -EACCES; - return false; - } + if (!parent_inode->metadata().may_write(*current)) + return KResult(-EACCES); - if (inode->directory_entry_count() != 2) { - error = -ENOTEMPTY; - return false; - } + if (inode->directory_entry_count() != 2) + return KResult(-ENOTEMPTY); - dbgprintf("VFS::rmdir: Removing inode %u:%u from parent %u:%u\n", inode->fsid(), inode->index(), parent_inode->fsid(), parent_inode->index()); + auto result = inode->remove_child("."); + if (result.is_error()) + return result; - // To do: - // - Remove '.' in target (--child.link_count) - // - Remove '..' in target (--parent.link_count) - // - Remove target from its parent (--parent.link_count) - if (!inode->remove_child(".", error)) - return false; - - if (!inode->remove_child("..", error)) - return false; + result = inode->remove_child(".."); + if (result.is_error()) + return result; // FIXME: The reverse_lookup here can definitely be avoided. - if (!parent_inode->remove_child(parent_inode->reverse_lookup(inode->identifier()), error)) - return false; - - error = 0; - return true; + return parent_inode->remove_child(parent_inode->reverse_lookup(inode->identifier())); } KResultOr VFS::resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode) diff --git a/Kernel/VirtualFileSystem.h b/Kernel/VirtualFileSystem.h index fdaa06d5ad..1345217824 100644 --- a/Kernel/VirtualFileSystem.h +++ b/Kernel/VirtualFileSystem.h @@ -67,8 +67,8 @@ public: RetainPtr create(const String& path, int& error, int options, mode_t mode, Inode& base); KResult mkdir(const String& path, mode_t mode, Inode& base); 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); + KResult unlink(const String& path, Inode& base); + KResult rmdir(const String& path, Inode& base); KResult chmod(const String& path, mode_t, Inode& base); KResult chown(const String& path, uid_t, gid_t, Inode& base); KResult access(const String& path, int mode, Inode& base);