LibCore: Make MappedFile a Stream

The internal reuse of FixedMemoryStream makes this straightforward.
There alread is one user of the new API, demonstrating the need for this
change beyond what I said out to use it for :^)

Tests/LibCore/CMakeLists.txt

@@ -3,6 +3,7 @@ set(TEST_SOURCES
     TestLibCoreDeferredInvoke.cpp
     TestLibCoreFilePermissionsMask.cpp
     TestLibCoreFileWatcher.cpp
+    TestLibCoreMappedFile.cpp
     TestLibCorePromise.cpp
     TestLibCoreSharedSingleProducerCircularQueue.cpp
     TestLibCoreStream.cpp

Tests/LibCore/TestLibCoreMappedFile.cpp

@@ -0,0 +1,218 @@
+/*
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
+ * Copyright (c) 2023, kleines Filmröllchen <filmroellchen@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <AK/Format.h>
+#include <AK/MaybeOwned.h>
+#include <AK/String.h>
+#include <LibCore/File.h>
+#include <LibCore/MappedFile.h>
+#include <LibTest/TestCase.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+TEST_CASE(mapped_file_open)
+{
+    // Fill the file with a little content so we can write to it.
+    constexpr auto text = "Here's some text to be mmapped."sv;
+    {
+        auto maybe_file = Core::File::open("/tmp/file-open-test.txt"sv, Core::File::OpenMode::Write);
+        if (maybe_file.is_error()) {
+            warnln("Failed to open the file: {}", strerror(maybe_file.error().code()));
+            VERIFY_NOT_REACHED();
+        }
+        TRY_OR_FAIL(maybe_file.value()->write_until_depleted(text.bytes()));
+    }
+
+    auto maybe_file = Core::MappedFile::map("/tmp/file-open-test.txt"sv, Core::MappedFile::OpenMode::ReadWrite);
+    if (maybe_file.is_error()) {
+        warnln("Failed to open the file: {}", strerror(maybe_file.error().code()));
+        VERIFY_NOT_REACHED();
+    }
+
+    // Testing out some basic file properties.
+    auto file = maybe_file.release_value();
+    EXPECT(file->is_open());
+    EXPECT(!file->is_eof());
+
+    auto size = TRY_OR_FAIL(file->size());
+    EXPECT_EQ(size, text.length());
+}
+
+constexpr auto expected_buffer_contents = "&lt;small&gt;(Please consider translating this message for the benefit of your fellow Wikimedians. Please also consider translating"sv;
+
+TEST_CASE(mapped_file_read_bytes)
+{
+    auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/long_lines.txt"sv, Core::MappedFile::OpenMode::ReadOnly));
+
+    auto buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(131));
+
+    auto result = file->read_some(buffer);
+    EXPECT_EQ(TRY_OR_FAIL(result).size(), 131ul);
+
+    StringView buffer_contents { buffer.bytes() };
+    EXPECT_EQ(buffer_contents, expected_buffer_contents);
+}
+
+constexpr auto expected_seek_contents1 = "|Lleer esti mens"sv;
+constexpr auto expected_seek_contents2 = "s of advanced ad"sv;
+constexpr auto expected_seek_contents3 = "levels of advanc"sv;
+
+TEST_CASE(mapped_file_seeking_around)
+{
+    auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/long_lines.txt"sv, Core::MappedFile::OpenMode::ReadOnly));
+
+    EXPECT_EQ(file->size().release_value(), 8702ul);
+
+    auto buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(16));
+
+    StringView buffer_contents { buffer.bytes() };
+
+    TRY_OR_FAIL(file->seek(500, SeekMode::SetPosition));
+    EXPECT_EQ(file->tell().release_value(), 500ul);
+    TRY_OR_FAIL(file->read_until_filled(buffer));
+    EXPECT_EQ(buffer_contents, expected_seek_contents1);
+
+    TRY_OR_FAIL(file->seek(234, SeekMode::FromCurrentPosition));
+    EXPECT_EQ(file->tell().release_value(), 750ul);
+    TRY_OR_FAIL(file->read_until_filled(buffer));
+    EXPECT_EQ(buffer_contents, expected_seek_contents2);
+
+    TRY_OR_FAIL(file->seek(-105, SeekMode::FromEndPosition));
+    EXPECT_EQ(file->tell().release_value(), 8597ul);
+    TRY_OR_FAIL(file->read_until_filled(buffer));
+    EXPECT_EQ(buffer_contents, expected_seek_contents3);
+}
+
+BENCHMARK_CASE(file_tell)
+{
+    auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/10kb.txt"sv, Core::MappedFile::OpenMode::ReadOnly));
+    auto expected_file_offset = 0u;
+    auto ten_byte_buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(1));
+    for (auto i = 0u; i < 4000; ++i) {
+        TRY_OR_FAIL(file->read_until_filled(ten_byte_buffer));
+        expected_file_offset += 1u;
+        EXPECT_EQ(expected_file_offset, TRY_OR_FAIL(file->tell()));
+    }
+
+    for (auto i = 0u; i < 4000; ++i) {
+        auto seek_file_offset = TRY_OR_FAIL(file->seek(-1, SeekMode::FromCurrentPosition));
+        expected_file_offset -= 1;
+        EXPECT_EQ(seek_file_offset, TRY_OR_FAIL(file->tell()));
+        EXPECT_EQ(expected_file_offset, TRY_OR_FAIL(file->tell()));
+    }
+}
+
+TEST_CASE(mapped_file_adopt_fd)
+{
+    int rc = ::open("/usr/Tests/LibCore/long_lines.txt", O_RDONLY);
+    EXPECT(rc >= 0);
+
+    auto file = TRY_OR_FAIL(Core::MappedFile::map_from_fd_and_close(rc, "/usr/Tests/LibCore/long_lines.txt"sv));
+
+    EXPECT_EQ(file->size().release_value(), 8702ul);
+
+    auto buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(16));
+
+    StringView buffer_contents { buffer.bytes() };
+
+    TRY_OR_FAIL(file->seek(500, SeekMode::SetPosition));
+    EXPECT_EQ(file->tell().release_value(), 500ul);
+    TRY_OR_FAIL(file->read_until_filled(buffer));
+    EXPECT_EQ(buffer_contents, expected_seek_contents1);
+
+    // A single seek & read test should be fine for now.
+}
+
+TEST_CASE(mapped_file_adopt_invalid_fd)
+{
+    auto maybe_file = Core::MappedFile::map_from_fd_and_close(-1, "/usr/Tests/LibCore/long_lines.txt"sv);
+    EXPECT(maybe_file.is_error());
+    EXPECT_EQ(maybe_file.error().code(), EBADF);
+}
+
+TEST_CASE(mapped_file_tell_and_seek)
+{
+    auto mapped_file = Core::MappedFile::map("/usr/Tests/LibCore/small.txt"sv).release_value();
+
+    // Initial state.
+    {
+        auto current_offset = mapped_file->tell().release_value();
+        EXPECT_EQ(current_offset, 0ul);
+    }
+
+    // Read a character.
+    {
+        auto character = mapped_file->read_value<char>().release_value();
+        EXPECT_EQ(character, 'W');
+        auto current_offset = mapped_file->tell().release_value();
+        EXPECT_EQ(current_offset, 1ul);
+    }
+
+    // Read one more character.
+    {
+        auto character = mapped_file->read_value<char>().release_value();
+        EXPECT_EQ(character, 'e');
+        auto current_offset = mapped_file->tell().release_value();
+        EXPECT_EQ(current_offset, 2ul);
+    }
+
+    // Seek seven characters forward.
+    {
+        auto current_offset = mapped_file->seek(7, SeekMode::FromCurrentPosition).release_value();
+        EXPECT_EQ(current_offset, 9ul);
+    }
+
+    // Read a character again.
+    {
+        auto character = mapped_file->read_value<char>().release_value();
+        EXPECT_EQ(character, 'o');
+        auto current_offset = mapped_file->tell().release_value();
+        EXPECT_EQ(current_offset, 10ul);
+    }
+
+    // Seek five characters backwards.
+    {
+        auto current_offset = mapped_file->seek(-5, SeekMode::FromCurrentPosition).release_value();
+        EXPECT_EQ(current_offset, 5ul);
+    }
+
+    // Read a character.
+    {
+        auto character = mapped_file->read_value<char>().release_value();
+        EXPECT_EQ(character, 'h');
+        auto current_offset = mapped_file->tell().release_value();
+        EXPECT_EQ(current_offset, 6ul);
+    }
+
+    // Seek back to the beginning.
+    {
+        auto current_offset = mapped_file->seek(0, SeekMode::SetPosition).release_value();
+        EXPECT_EQ(current_offset, 0ul);
+    }
+
+    // Read the first character. This should prime the buffer if it hasn't happened already.
+    {
+        auto character = mapped_file->read_value<char>().release_value();
+        EXPECT_EQ(character, 'W');
+        auto current_offset = mapped_file->tell().release_value();
+        EXPECT_EQ(current_offset, 1ul);
+    }
+
+    // Seek beyond the buffer size, which should invalidate the buffer.
+    {
+        auto current_offset = mapped_file->seek(12, SeekMode::SetPosition).release_value();
+        EXPECT_EQ(current_offset, 12ul);
+    }
+
+    // Ensure that we still read the correct contents from the new offset with a (presumably) freshly filled buffer.
+    {
+        auto character = mapped_file->read_value<char>().release_value();
+        EXPECT_EQ(character, 'r');
+        auto current_offset = mapped_file->tell().release_value();
+        EXPECT_EQ(current_offset, 13ul);
+    }
+}

Userland/Libraries/LibCore/MappedFile.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2023, kleines Filmröllchen <filmroellchen@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -14,10 +15,11 @@
 
 namespace Core {
 
-ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map(StringView path)
+ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map(StringView path, OpenMode mode)
 {
-    auto fd = TRY(Core::System::open(path, O_RDONLY | O_CLOEXEC, 0));
+    auto const file_mode = mode == OpenMode::ReadOnly ? O_RDONLY : O_RDWR;
-    return map_from_fd_and_close(fd, path);
+    auto fd = TRY(Core::System::open(path, file_mode | O_CLOEXEC, 0));
+    return map_from_fd_and_close(fd, path, mode);
 }
 
 ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_file(NonnullOwnPtr<Core::File> stream, StringView path)
@@ -25,24 +27,39 @@ ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_file(NonnullOwnPtr<Core:
     return map_from_fd_and_close(stream->leak_fd(Badge<MappedFile> {}), path);
 }
 
-ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_fd_and_close(int fd, [[maybe_unused]] StringView path)
+ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_fd_and_close(int fd, [[maybe_unused]] StringView path, OpenMode mode)
 {
     TRY(Core::System::fcntl(fd, F_SETFD, FD_CLOEXEC));
 
     ScopeGuard fd_close_guard = [fd] {
-        close(fd);
+        ::close(fd);
     };
 
     auto stat = TRY(Core::System::fstat(fd));
     auto size = stat.st_size;
 
-    auto* ptr = TRY(Core::System::mmap(nullptr, size, PROT_READ, MAP_SHARED, fd, 0, 0, path));
+    int protection;
+    int flags;
+    switch (mode) {
+    case OpenMode::ReadOnly:
+        protection = PROT_READ;
+        flags = MAP_SHARED;
+        break;
+    case OpenMode::ReadWrite:
+        protection = PROT_READ | PROT_WRITE;
+        // Don't map a read-write mapping shared as a precaution.
+        flags = MAP_PRIVATE;
+        break;
+    }
 
-    return adopt_own(*new MappedFile(ptr, size));
+    auto* ptr = TRY(Core::System::mmap(nullptr, size, protection, flags, fd, 0, 0, path));
+
+    return adopt_own(*new MappedFile(ptr, size, mode));
 }
 
-MappedFile::MappedFile(void* ptr, size_t size)
+MappedFile::MappedFile(void* ptr, size_t size, OpenMode mode)
-    : m_data(ptr)
+    : FixedMemoryStream(Bytes { ptr, size }, mode == OpenMode::ReadWrite)
+    , m_data(ptr)
     , m_size(size)
 {
 }

Userland/Libraries/LibCore/MappedFile.h

@@ -7,7 +7,9 @@
 
 #pragma once
 
+#include <AK/ByteBuffer.h>
 #include <AK/Error.h>
+#include <AK/MemoryStream.h>
 #include <AK/Noncopyable.h>
 #include <AK/NonnullOwnPtr.h>
 #include <AK/RefCounted.h>
@@ -15,25 +17,29 @@
 
 namespace Core {
 
-class MappedFile {
+class MappedFile : public FixedMemoryStream {
     AK_MAKE_NONCOPYABLE(MappedFile);
     AK_MAKE_NONMOVABLE(MappedFile);
 
 public:
-    static ErrorOr<NonnullOwnPtr<MappedFile>> map(StringView path);
+    // Reflects a simplified version of mmap protection and flags.
-    static ErrorOr<NonnullOwnPtr<MappedFile>> map_from_file(NonnullOwnPtr<Core::File>, StringView path);
+    enum class OpenMode {
-    static ErrorOr<NonnullOwnPtr<MappedFile>> map_from_fd_and_close(int fd, StringView path);
+        ReadOnly,
-    ~MappedFile();
+        ReadWrite,
+    };
 
+    static ErrorOr<NonnullOwnPtr<MappedFile>> map(StringView path, OpenMode mode = OpenMode::ReadOnly);
+    static ErrorOr<NonnullOwnPtr<MappedFile>> map_from_file(NonnullOwnPtr<Core::File>, StringView path);
+    static ErrorOr<NonnullOwnPtr<MappedFile>> map_from_fd_and_close(int fd, StringView path, OpenMode mode = OpenMode::ReadOnly);
+    virtual ~MappedFile();
+
+    // Non-stream APIs for using MappedFile as a simple POSIX API wrapper.
     void* data() { return m_data; }
     void const* data() const { return m_data; }
-
-    size_t size() const { return m_size; }
-
     ReadonlyBytes bytes() const { return { m_data, m_size }; }
 
 private:
-    explicit MappedFile(void*, size_t);
+    explicit MappedFile(void*, size_t, OpenMode);
 
     void* m_data { nullptr };
     size_t m_size { 0 };

Userland/Libraries/LibGUI/FileIconProvider.cpp

@@ -174,7 +174,7 @@ Icon FileIconProvider::icon_for_executable(DeprecatedString const& path)
 
     auto& mapped_file = file_or_error.value();
 
-    if (mapped_file->size() < SELFMAG) {
+    if (mapped_file->size().release_value() < SELFMAG) {
         app_icon_cache.set(path, s_executable_icon);
         return s_executable_icon;
     }
@@ -184,7 +184,7 @@ Icon FileIconProvider::icon_for_executable(DeprecatedString const& path)
         return s_executable_icon;
     }
 
-    auto image = ELF::Image((u8 const*)mapped_file->data(), mapped_file->size());
+    auto image = ELF::Image((u8 const*)mapped_file->data(), mapped_file->size().release_value());
     if (!image.is_valid()) {
         app_icon_cache.set(path, s_executable_icon);
         return s_executable_icon;

Userland/Utilities/disasm.cpp

@@ -29,13 +29,13 @@ ErrorOr<int> serenity_main(Main::Arguments args)
     args_parser.add_positional_argument(path, "Path to i386 binary file", "path");
     args_parser.parse(args);
 
-    OwnPtr<Core::MappedFile const> file;
+    OwnPtr<Core::MappedFile> file;
     u8 const* asm_data = nullptr;
     size_t asm_size = 0;
     if ((TRY(Core::System::stat(path))).st_size > 0) {
         file = TRY(Core::MappedFile::map(path));
         asm_data = static_cast<u8 const*>(file->data());
-        asm_size = file->size();
+        asm_size = MUST(file->size());
     }
 
     struct Symbol {

Userland/Utilities/fdtdump.cpp

@@ -25,7 +25,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     // FIXME: Figure out how to do this sanely from stdin
     auto file = TRY(Core::MappedFile::map(filename));
 
-    if (file->size() < sizeof(DeviceTree::FlattenedDeviceTreeHeader)) {
+    if (TRY(file->size()) < sizeof(DeviceTree::FlattenedDeviceTreeHeader)) {
         warnln("Not enough data in {} to contain a device tree header!", filename);
         return 1;
     }

Userland/Utilities/wasm.cpp

@@ -258,8 +258,7 @@ static Optional<Wasm::Module> parse(StringView filename)
         return {};
     }
 
-    FixedMemoryStream stream { ReadonlyBytes { result.value()->data(), result.value()->size() } };
+    auto parse_result = Wasm::Module::parse(*result.value());
-    auto parse_result = Wasm::Module::parse(stream);
     if (parse_result.is_error()) {
         warnln("Something went wrong, either the file is invalid, or there's a bug with LibWasm!");
         warnln("The parse error was {}", Wasm::parse_error_to_deprecated_string(parse_result.error()));