diff --git a/Tests/LibCore/CMakeLists.txt b/Tests/LibCore/CMakeLists.txt index d0455d2c04..42bc7dece4 100644 --- a/Tests/LibCore/CMakeLists.txt +++ b/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 diff --git a/Tests/LibCore/TestLibCoreMappedFile.cpp b/Tests/LibCore/TestLibCoreMappedFile.cpp new file mode 100644 index 0000000000..8f2aeb33c5 --- /dev/null +++ b/Tests/LibCore/TestLibCoreMappedFile.cpp @@ -0,0 +1,218 @@ +/* + * Copyright (c) 2021, sin-ack + * Copyright (c) 2023, kleines Filmröllchen + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +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 = "<small>(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().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().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().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().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().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().release_value(); + EXPECT_EQ(character, 'r'); + auto current_offset = mapped_file->tell().release_value(); + EXPECT_EQ(current_offset, 13ul); + } +} diff --git a/Userland/Libraries/LibCore/MappedFile.cpp b/Userland/Libraries/LibCore/MappedFile.cpp index 125d5b0c08..4d8262ea36 100644 --- a/Userland/Libraries/LibCore/MappedFile.cpp +++ b/Userland/Libraries/LibCore/MappedFile.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2018-2021, Andreas Kling + * Copyright (c) 2023, kleines Filmröllchen * * SPDX-License-Identifier: BSD-2-Clause */ @@ -14,10 +15,11 @@ namespace Core { -ErrorOr> MappedFile::map(StringView path) +ErrorOr> MappedFile::map(StringView path, OpenMode mode) { - auto fd = TRY(Core::System::open(path, O_RDONLY | O_CLOEXEC, 0)); - return map_from_fd_and_close(fd, path); + auto const file_mode = mode == OpenMode::ReadOnly ? O_RDONLY : O_RDWR; + auto fd = TRY(Core::System::open(path, file_mode | O_CLOEXEC, 0)); + return map_from_fd_and_close(fd, path, mode); } ErrorOr> MappedFile::map_from_file(NonnullOwnPtr stream, StringView path) @@ -25,24 +27,39 @@ ErrorOr> MappedFile::map_from_file(NonnullOwnPtrleak_fd(Badge {}), path); } -ErrorOr> MappedFile::map_from_fd_and_close(int fd, [[maybe_unused]] StringView path) +ErrorOr> 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) - : m_data(ptr) +MappedFile::MappedFile(void* ptr, size_t size, OpenMode mode) + : FixedMemoryStream(Bytes { ptr, size }, mode == OpenMode::ReadWrite) + , m_data(ptr) , m_size(size) { } diff --git a/Userland/Libraries/LibCore/MappedFile.h b/Userland/Libraries/LibCore/MappedFile.h index 8372c6e4bd..1863b14e41 100644 --- a/Userland/Libraries/LibCore/MappedFile.h +++ b/Userland/Libraries/LibCore/MappedFile.h @@ -7,7 +7,9 @@ #pragma once +#include #include +#include #include #include #include @@ -15,25 +17,29 @@ namespace Core { -class MappedFile { +class MappedFile : public FixedMemoryStream { AK_MAKE_NONCOPYABLE(MappedFile); AK_MAKE_NONMOVABLE(MappedFile); public: - static ErrorOr> map(StringView path); - static ErrorOr> map_from_file(NonnullOwnPtr, StringView path); - static ErrorOr> map_from_fd_and_close(int fd, StringView path); - ~MappedFile(); + // Reflects a simplified version of mmap protection and flags. + enum class OpenMode { + ReadOnly, + ReadWrite, + }; + static ErrorOr> map(StringView path, OpenMode mode = OpenMode::ReadOnly); + static ErrorOr> map_from_file(NonnullOwnPtr, StringView path); + static ErrorOr> 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 }; diff --git a/Userland/Libraries/LibGUI/FileIconProvider.cpp b/Userland/Libraries/LibGUI/FileIconProvider.cpp index b4bbadcf6a..d3fa873e63 100644 --- a/Userland/Libraries/LibGUI/FileIconProvider.cpp +++ b/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; diff --git a/Userland/Utilities/disasm.cpp b/Userland/Utilities/disasm.cpp index 1b00c6aa34..0703c81a80 100644 --- a/Userland/Utilities/disasm.cpp +++ b/Userland/Utilities/disasm.cpp @@ -29,13 +29,13 @@ ErrorOr serenity_main(Main::Arguments args) args_parser.add_positional_argument(path, "Path to i386 binary file", "path"); args_parser.parse(args); - OwnPtr file; + OwnPtr 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(file->data()); - asm_size = file->size(); + asm_size = MUST(file->size()); } struct Symbol { diff --git a/Userland/Utilities/fdtdump.cpp b/Userland/Utilities/fdtdump.cpp index b49e869ca8..106c9997ba 100644 --- a/Userland/Utilities/fdtdump.cpp +++ b/Userland/Utilities/fdtdump.cpp @@ -25,7 +25,7 @@ ErrorOr 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; } diff --git a/Userland/Utilities/wasm.cpp b/Userland/Utilities/wasm.cpp index 01d6cf2e5a..0f2f5373d3 100644 --- a/Userland/Utilities/wasm.cpp +++ b/Userland/Utilities/wasm.cpp @@ -258,8 +258,7 @@ static Optional parse(StringView filename) return {}; } - FixedMemoryStream stream { ReadonlyBytes { result.value()->data(), result.value()->size() } }; - auto parse_result = Wasm::Module::parse(stream); + auto parse_result = Wasm::Module::parse(*result.value()); 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()));