Browse Source

Kernel: Improvements to Custody absolute path serialization

- Renamed try_create_absolute_path() => try_serialize_absolute_path()
- Use KResultOr and TRY() to propagate errors
- Don't call this when it's only for debug logging
Andreas Kling 3 years ago
parent
commit
cda2b9e71c

+ 9 - 5
Kernel/FileSystem/Custody.cpp

@@ -71,10 +71,14 @@ Custody::~Custody()
 {
 }
 
-OwnPtr<KString> Custody::try_create_absolute_path() const
+KResultOr<NonnullOwnPtr<KString>> Custody::try_serialize_absolute_path() const
 {
-    if (!parent())
-        return KString::try_create("/"sv);
+    if (!parent()) {
+        auto string = KString::try_create("/"sv);
+        if (!string)
+            return ENOMEM;
+        return string.release_nonnull();
+    }
 
     Vector<Custody const*, 32> custody_chain;
     size_t path_length = 0;
@@ -87,7 +91,7 @@ OwnPtr<KString> Custody::try_create_absolute_path() const
     char* buffer;
     auto string = KString::try_create_uninitialized(path_length - 1, buffer);
     if (!string)
-        return string;
+        return ENOMEM;
     size_t string_index = 0;
     for (size_t custody_index = custody_chain.size() - 1; custody_index > 0; --custody_index) {
         buffer[string_index] = '/';
@@ -98,7 +102,7 @@ OwnPtr<KString> Custody::try_create_absolute_path() const
     }
     VERIFY(string->length() == string_index);
     buffer[string_index] = 0;
-    return string;
+    return string.release_nonnull();
 }
 
 String Custody::absolute_path() const

+ 1 - 1
Kernel/FileSystem/Custody.h

@@ -33,7 +33,7 @@ public:
     Inode& inode() { return *m_inode; }
     Inode const& inode() const { return *m_inode; }
     StringView name() const { return m_name->view(); }
-    OwnPtr<KString> try_create_absolute_path() const;
+    KResultOr<NonnullOwnPtr<KString>> try_serialize_absolute_path() const;
     String absolute_path() const;
 
     int mount_flags() const { return m_mount_flags; }

+ 5 - 10
Kernel/FileSystem/VirtualFileSystem.cpp

@@ -54,9 +54,8 @@ KResult VirtualFileSystem::mount(FileSystem& fs, Custody& mount_point, int flags
 {
     return m_mounts.with_exclusive([&](auto& mounts) -> KResult {
         auto& inode = mount_point.inode();
-        dbgln("VirtualFileSystem: Mounting {} at {} (inode: {}) with flags {}",
+        dbgln("VirtualFileSystem: Mounting {} at inode {} with flags {}",
             fs.class_name(),
-            mount_point.try_create_absolute_path(),
             inode.identifier(),
             flags);
         // FIXME: check that this is not already a mount point
@@ -69,7 +68,7 @@ KResult VirtualFileSystem::mount(FileSystem& fs, Custody& mount_point, int flags
 KResult VirtualFileSystem::bind_mount(Custody& source, Custody& mount_point, int flags)
 {
     return m_mounts.with_exclusive([&](auto& mounts) -> KResult {
-        dbgln("VirtualFileSystem: Bind-mounting {} at {}", source.try_create_absolute_path(), mount_point.try_create_absolute_path());
+        dbgln("VirtualFileSystem: Bind-mounting inode {} at inode {}", source.inode().identifier(), mount_point.inode().identifier());
         // FIXME: check that this is not already a mount point
         Mount mount { source.inode(), mount_point, flags };
         mounts.append(move(mount));
@@ -79,7 +78,7 @@ KResult VirtualFileSystem::bind_mount(Custody& source, Custody& mount_point, int
 
 KResult VirtualFileSystem::remount(Custody& mount_point, int new_flags)
 {
-    dbgln("VirtualFileSystem: Remounting {}", mount_point.try_create_absolute_path());
+    dbgln("VirtualFileSystem: Remounting inode {}", mount_point.inode().identifier());
 
     auto* mount = find_mount_for_guest(mount_point.inode().identifier());
     if (!mount)
@@ -323,9 +322,7 @@ KResult VirtualFileSystem::mknod(StringView path, mode_t mode, dev_t dev, Custod
 KResultOr<NonnullRefPtr<FileDescription>> VirtualFileSystem::create(StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> owner)
 {
     auto basename = KLexicalPath::basename(path);
-    auto parent_path = parent_custody.try_create_absolute_path();
-    if (!parent_path)
-        return ENOMEM;
+    auto parent_path = TRY(parent_custody.try_serialize_absolute_path());
     auto full_path = KLexicalPath::try_join(parent_path->view(), basename);
     if (!full_path)
         return ENOMEM;
@@ -764,9 +761,7 @@ KResult VirtualFileSystem::validate_path_against_process_veil(Custody const& cus
 {
     if (Process::current().veil_state() == VeilState::None)
         return KSuccess;
-    auto absolute_path = custody.try_create_absolute_path();
-    if (!absolute_path)
-        return ENOMEM;
+    auto absolute_path = TRY(custody.try_serialize_absolute_path());
     return validate_path_against_process_veil(absolute_path->view(), options);
 }
 

+ 3 - 7
Kernel/Syscalls/chdir.cpp

@@ -40,14 +40,10 @@ KResultOr<FlatPtr> Process::sys$getcwd(Userspace<char*> buffer, size_t size)
     if (size > NumericLimits<ssize_t>::max())
         return EINVAL;
 
-    auto maybe_path = current_directory().try_create_absolute_path();
-    if (!maybe_path)
-        return ENOMEM;
-    auto& path = *maybe_path;
-
-    size_t ideal_size = path.length() + 1;
+    auto path = TRY(current_directory().try_serialize_absolute_path());
+    size_t ideal_size = path->length() + 1;
     auto size_to_copy = min(ideal_size, size);
-    TRY(copy_to_user(buffer, path.characters(), size_to_copy));
+    TRY(copy_to_user(buffer, path->characters(), size_to_copy));
     // Note: we return the whole size here, not the copied size.
     return ideal_size;
 }

+ 1 - 4
Kernel/Syscalls/realpath.cpp

@@ -19,10 +19,7 @@ KResultOr<FlatPtr> Process::sys$realpath(Userspace<const Syscall::SC_realpath_pa
 
     auto path = TRY(get_syscall_path_argument(params.path));
     auto custody = TRY(VirtualFileSystem::the().resolve_path(path->view(), current_directory()));
-
-    auto absolute_path = custody->try_create_absolute_path();
-    if (!absolute_path)
-        return ENOMEM;
+    auto absolute_path = TRY(custody->try_serialize_absolute_path());
 
     size_t ideal_size = absolute_path->length() + 1;
     auto size_to_copy = min(ideal_size, params.buffer.size);

+ 2 - 6
Kernel/Syscalls/unveil.cpp

@@ -83,13 +83,9 @@ KResultOr<FlatPtr> Process::sys$unveil(Userspace<const Syscall::SC_unveil_params
     OwnPtr<KString> new_unveiled_path;
     auto custody_or_error = VirtualFileSystem::the().resolve_path_without_veil(path->view(), VirtualFileSystem::the().root_custody(), &parent_custody);
     if (!custody_or_error.is_error()) {
-        new_unveiled_path = custody_or_error.value()->try_create_absolute_path();
-        if (!new_unveiled_path)
-            return ENOMEM;
+        new_unveiled_path = TRY(custody_or_error.value()->try_serialize_absolute_path());
     } else if (custody_or_error.error() == ENOENT && parent_custody && (new_permissions & UnveilAccess::CreateOrRemove)) {
-        auto parent_custody_path = parent_custody->try_create_absolute_path();
-        if (!parent_custody_path)
-            return ENOMEM;
+        auto parent_custody_path = TRY(parent_custody->try_serialize_absolute_path());
         new_unveiled_path = KLexicalPath::try_join(parent_custody_path->view(), KLexicalPath::basename(path->view()));
         if (!new_unveiled_path)
             return ENOMEM;