瀏覽代碼

Kernel: Use KResult in unlink() and rmdir().

Andreas Kling 6 年之前
父節點
當前提交
5b27f11b97

+ 9 - 17
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<DirectoryEntry>&& entries)

+ 1 - 1
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<Inode> parent() const override;
     virtual int set_atime(time_t) override;
     virtual int set_ctime(time_t) override;

+ 1 - 1
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<Inode> parent() const = 0;
     virtual size_t directory_entry_count() const = 0;
     virtual KResult chmod(mode_t) = 0;

+ 3 - 0
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<typename T> friend class KResultOr;
     KResult() { }

+ 2 - 3
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()

+ 1 - 1
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<Inode> parent() const override;
     virtual size_t directory_entry_count() const override;
     virtual KResult chmod(mode_t) override;

+ 2 - 8
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)

+ 2 - 4
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()

+ 1 - 1
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<Inode> parent() const override;
     virtual size_t directory_entry_count() const override;
     virtual KResult chmod(mode_t) override;

+ 30 - 54
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<Inode> parent_inode;
-    auto inode = resolve_path_to_inode(path, base, error, &parent_inode);
-    if (!inode)
-        return false;
-
-    if (inode->is_directory()) {
-        error = -EISDIR;
-        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 (!parent_inode->metadata().may_write(*current)) {
-        error = -EACCES;
-        return false;
-    }
+    if (inode->is_directory())
+        return KResult(-EISDIR);
 
-    if (!parent_inode->remove_child(FileSystemPath(path).basename(), error))
-        return false;
+    if (!parent_inode->metadata().may_write(*current))
+        return KResult(-EACCES);
 
-    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<Inode> 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 (!parent_inode->metadata().may_write(*current)) {
-        error = -EACCES;
-        return false;
-    }
+    if (!inode->is_directory())
+        return KResult(-ENOTDIR);
 
-    if (inode->directory_entry_count() != 2) {
-        error = -ENOTEMPTY;
-        return false;
-    }
+    if (!parent_inode->metadata().may_write(*current))
+        return KResult(-EACCES);
 
-    dbgprintf("VFS::rmdir: Removing inode %u:%u from parent %u:%u\n", inode->fsid(), inode->index(), parent_inode->fsid(), parent_inode->index());
+    if (inode->directory_entry_count() != 2)
+        return KResult(-ENOTEMPTY);
 
-    // 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;
+    auto result = inode->remove_child(".");
+    if (result.is_error())
+        return result;
 
-    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<InodeIdentifier> VFS::resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode)

+ 2 - 2
Kernel/VirtualFileSystem.h

@@ -67,8 +67,8 @@ public:
     RetainPtr<FileDescriptor> 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);