Explorar el Código

Kernel: Make Inode::read_entire() return a KBuffer (not ByteBuffer)

ByteBuffer is backed by kmalloc heap memory which is a scarce resource.
This fixes an OOM panic when traversing a large directory.
Andreas Kling hace 4 años
padre
commit
62ec6e5fe0

+ 1 - 1
Kernel/FileSystem/FileDescription.cpp

@@ -163,7 +163,7 @@ bool FileDescription::can_read() const
     return m_file->can_read(*this, offset());
 }
 
-KResultOr<ByteBuffer> FileDescription::read_entire_file()
+KResultOr<KBuffer> FileDescription::read_entire_file()
 {
     // HACK ALERT: (This entire function)
     ASSERT(m_file->is_inode());

+ 1 - 1
Kernel/FileSystem/FileDescription.h

@@ -71,7 +71,7 @@ public:
 
     ssize_t get_dir_entries(u8* buffer, ssize_t);
 
-    KResultOr<ByteBuffer> read_entire_file();
+    KResultOr<KBuffer> read_entire_file();
 
     String absolute_path() const;
 

+ 5 - 11
Kernel/FileSystem/Inode.cpp

@@ -31,6 +31,7 @@
 #include <Kernel/FileSystem/Inode.h>
 #include <Kernel/FileSystem/InodeWatcher.h>
 #include <Kernel/FileSystem/VirtualFileSystem.h>
+#include <Kernel/KBufferBuilder.h>
 #include <Kernel/Net/LocalSocket.h>
 #include <Kernel/VM/SharedInodeVMObject.h>
 
@@ -65,10 +66,9 @@ void Inode::sync()
     }
 }
 
-KResultOr<ByteBuffer> Inode::read_entire(FileDescription* descriptor) const
+KResultOr<KBuffer> Inode::read_entire(FileDescription* descriptor) const
 {
-    size_t initial_size = metadata().size ? metadata().size : 4096;
-    StringBuilder builder(initial_size);
+    KBufferBuilder builder;
 
     ssize_t nread;
     u8 buffer[4096];
@@ -85,10 +85,10 @@ KResultOr<ByteBuffer> Inode::read_entire(FileDescription* descriptor) const
     }
     if (nread < 0) {
         klog() << "Inode::read_entire: ERROR: " << nread;
-        return nullptr;
+        return KResult(nread);
     }
 
-    return builder.to_byte_buffer();
+    return builder.build();
 }
 
 KResultOr<NonnullRefPtr<Custody>> Inode::resolve_as_link(Custody& base, RefPtr<Custody>* out_parent, int options, int symlink_recursion_level) const
@@ -101,12 +101,6 @@ KResultOr<NonnullRefPtr<Custody>> Inode::resolve_as_link(Custody& base, RefPtr<C
         return contents_or.error();
 
     auto& contents = contents_or.value();
-    if (!contents) {
-        if (out_parent)
-            *out_parent = nullptr;
-        return KResult(-ENOENT);
-    }
-
     auto path = StringView(contents.data(), contents.size());
     return VFS::the().resolve_path(path, base, out_parent, options, symlink_recursion_level);
 }

+ 1 - 1
Kernel/FileSystem/Inode.h

@@ -67,7 +67,7 @@ public:
     InodeIdentifier identifier() const { return { fsid(), index() }; }
     virtual InodeMetadata metadata() const = 0;
 
-    KResultOr<ByteBuffer> read_entire(FileDescription* = nullptr) const;
+    KResultOr<KBuffer> read_entire(FileDescription* = nullptr) const;
 
     virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const = 0;
     virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const = 0;

+ 3 - 0
Kernel/KBuffer.h

@@ -105,6 +105,9 @@ public:
     size_t size() const { return m_impl->size(); }
     size_t capacity() const { return m_impl->capacity(); }
 
+    void* end_pointer() { return data() + size(); }
+    const void* end_pointer() const { return data() + size(); }
+
     void set_size(size_t size) { m_impl->set_size(size); }
 
     const KBufferImpl& impl() const { return m_impl; }

+ 1 - 1
Kernel/KSyms.cpp

@@ -69,7 +69,7 @@ const KernelSymbol* symbolicate_kernel_address(u32 address)
     return nullptr;
 }
 
-static void load_kernel_sybols_from_data(const ByteBuffer& buffer)
+static void load_kernel_sybols_from_data(const KBuffer& buffer)
 {
     g_lowest_kernel_symbol_address = 0xffffffff;
     g_highest_kernel_symbol_address = 0;

+ 0 - 1
Kernel/Syscalls/module.cpp

@@ -56,7 +56,6 @@ int Process::sys$module_load(Userspace<const char*> user_path, size_t path_lengt
     auto payload = payload_or_error.value();
     auto storage = KBuffer::create_with_size(payload.size());
     memcpy(storage.data(), payload.data(), payload.size());
-    payload.clear();
 
     auto elf_image = make<ELF::Image>(storage.data(), storage.size());
     if (!elf_image->parse())