From 2f3b901f7fe230cbbf489f5a0f38fbb34a07af8c Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 10 Jan 2021 15:55:54 +0100 Subject: [PATCH] AK: Make MappedFile heap-allocated and ref-counted Let's adapt this class a bit better to how it's actually being used. Instead of having valid/invalid states and storing an error in case it's invalid, a MappedFile is now always valid, and the factory function that creates it will return an OSError if mapping fails. --- AK/MappedFile.cpp | 74 +++++++------------ AK/MappedFile.h | 32 ++++---- Applications/Help/ManualModel.cpp | 22 +++--- Applications/Help/ManualModel.h | 4 +- Applications/Help/main.cpp | 2 +- .../HackStudio/Debugger/DisassemblyModel.cpp | 6 +- DevTools/Profiler/DisassemblyModel.cpp | 5 +- DevTools/Profiler/DisassemblyModel.h | 2 +- DevTools/Profiler/Profile.cpp | 6 +- DevTools/UserspaceEmulator/Emulator.cpp | 34 +++++---- DevTools/UserspaceEmulator/Emulator.h | 2 +- Libraries/LibCoreDump/Backtrace.cpp | 7 +- Libraries/LibCoreDump/Backtrace.h | 4 +- Libraries/LibCoreDump/Reader.cpp | 18 ++--- Libraries/LibCoreDump/Reader.h | 9 ++- Libraries/LibDebug/DebugSession.cpp | 15 +--- Libraries/LibDebug/DebugSession.h | 6 +- Libraries/LibELF/Image.cpp | 11 ++- Libraries/LibELF/Image.h | 2 + Libraries/LibGUI/FileIconProvider.cpp | 6 +- Libraries/LibGUI/ImageWidget.cpp | 5 +- Libraries/LibGfx/BMPLoader.cpp | 6 +- Libraries/LibGfx/BitmapFont.cpp | 9 +-- Libraries/LibGfx/BitmapFont.h | 2 +- Libraries/LibGfx/GIFLoader.cpp | 6 +- Libraries/LibGfx/ICOLoader.cpp | 6 +- Libraries/LibGfx/JPGLoader.cpp | 8 +- Libraries/LibGfx/PNGLoader.cpp | 6 +- Libraries/LibGfx/PortableImageLoaderCommon.h | 8 +- Libraries/LibPCIDB/Database.cpp | 10 +-- Libraries/LibPCIDB/Database.h | 8 +- Services/WebServer/Client.cpp | 10 ++- Userland/disasm.cpp | 8 +- Userland/functrace.cpp | 2 +- Userland/readelf.cpp | 15 ++-- Userland/unzip.cpp | 7 +- 36 files changed, 184 insertions(+), 199 deletions(-) diff --git a/AK/MappedFile.cpp b/AK/MappedFile.cpp index 9153475e7a..b855a42cc7 100644 --- a/AK/MappedFile.cpp +++ b/AK/MappedFile.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2018-2021, Andreas Kling * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -25,6 +25,7 @@ */ #include +#include #include #include #include @@ -32,68 +33,43 @@ #include #include -//#define DEBUG_MAPPED_FILE - namespace AK { -MappedFile::MappedFile(const StringView& file_name) +Result, OSError> MappedFile::map(const StringView& path) { - int fd = open_with_path_length(file_name.characters_without_null_termination(), file_name.length(), O_RDONLY | O_CLOEXEC, 0); + int fd = open_with_path_length(path.characters_without_null_termination(), path.length(), O_RDONLY | O_CLOEXEC, 0); + if (fd < 0) + return OSError(errno); - if (fd == -1) { - m_errno = errno; - perror("open"); - return; - } + ScopeGuard fd_close_guard = [fd] { + close(fd); + }; struct stat st; - fstat(fd, &st); - m_size = st.st_size; - m_map = mmap(nullptr, m_size, PROT_READ, MAP_SHARED, fd, 0); - - if (m_map == MAP_FAILED) { - m_errno = errno; - perror("mmap"); + if (fstat(fd, &st) < 0) { + auto saved_errno = errno; + return OSError(saved_errno); } -#ifdef DEBUG_MAPPED_FILE - dbgln("MappedFile(\"{}\") := ( fd={}, m_size={}, m_map={} )", file_name, fd, m_size, m_map); -#endif + auto size = st.st_size; + auto* ptr = mmap(nullptr, size, PROT_READ, MAP_SHARED, fd, 0); - close(fd); + if (ptr == MAP_FAILED) + return OSError(errno); + + return adopt(*new MappedFile(ptr, size)); +} + +MappedFile::MappedFile(void* ptr, size_t size) + : m_data(ptr) + , m_size(size) +{ } MappedFile::~MappedFile() { - unmap(); -} - -void MappedFile::unmap() -{ - if (!is_valid()) - return; - int rc = munmap(m_map, m_size); + auto rc = munmap(m_data, m_size); ASSERT(rc == 0); - m_size = 0; - m_map = (void*)-1; -} - -MappedFile::MappedFile(MappedFile&& other) - : m_size(other.m_size) - , m_map(other.m_map) -{ - other.m_size = 0; - other.m_map = (void*)-1; -} - -MappedFile& MappedFile::operator=(MappedFile&& other) -{ - if (this == &other) - return *this; - unmap(); - swap(m_size, other.m_size); - swap(m_map, other.m_map); - return *this; } } diff --git a/AK/MappedFile.h b/AK/MappedFile.h index a7649e78a5..b3bd099cdd 100644 --- a/AK/MappedFile.h +++ b/AK/MappedFile.h @@ -27,37 +27,33 @@ #pragma once #include -#include +#include +#include +#include +#include namespace AK { -class MappedFile { +class MappedFile : public RefCounted { AK_MAKE_NONCOPYABLE(MappedFile); + AK_MAKE_NONMOVABLE(MappedFile); public: - MappedFile() { } - explicit MappedFile(const StringView& file_name); - MappedFile(MappedFile&&); + static Result, OSError> map(const StringView& path); ~MappedFile(); - MappedFile& operator=(MappedFile&&); + void* data() { return m_data; } + const void* data() const { return m_data; } - bool is_valid() const { return m_map != (void*)-1; } - int errno_if_invalid() const - { - ASSERT(!is_valid()); - return m_errno; - } - void unmap(); - - void* data() { return m_map; } - const void* data() const { return m_map; } size_t size() const { return m_size; } + ReadonlyBytes bytes() const { return { m_data, m_size }; } + private: + explicit MappedFile(void*, size_t); + + void* m_data { nullptr }; size_t m_size { 0 }; - void* m_map { (void*)-1 }; - int m_errno { 0 }; }; } diff --git a/Applications/Help/ManualModel.cpp b/Applications/Help/ManualModel.cpp index ee3fae6b17..484b1b22c1 100644 --- a/Applications/Help/ManualModel.cpp +++ b/Applications/Help/ManualModel.cpp @@ -79,22 +79,24 @@ String ManualModel::page_path(const GUI::ModelIndex& index) const return page->path(); } -Result ManualModel::page_view(const String& path) const +Result ManualModel::page_view(const String& path) const { if (path.is_empty()) return StringView {}; - auto mapped_file = m_mapped_files.get(path); - if (mapped_file.has_value()) - return StringView { (const char*)mapped_file.value()->data(), mapped_file.value()->size() }; + { + // Check if we've got it cached already. + auto mapped_file = m_mapped_files.get(path); + if (mapped_file.has_value()) + return StringView { mapped_file.value()->bytes() }; + } - auto map = make(path); - if (!map->is_valid()) - return map->errno_if_invalid(); - - StringView view { (const char*)map->data(), map->size() }; - m_mapped_files.set(path, move(map)); + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) + return file_or_error.error(); + StringView view { file_or_error.value()->bytes() }; + m_mapped_files.set(path, file_or_error.release_value()); return view; } diff --git a/Applications/Help/ManualModel.h b/Applications/Help/ManualModel.h index f2a9c47122..02efecaff0 100644 --- a/Applications/Help/ManualModel.h +++ b/Applications/Help/ManualModel.h @@ -45,7 +45,7 @@ public: String page_path(const GUI::ModelIndex&) const; String page_and_section(const GUI::ModelIndex&) const; - Result page_view(const String& path) const; + Result page_view(const String& path) const; void update_section_node_on_toggle(const GUI::ModelIndex&, const bool); virtual int row_count(const GUI::ModelIndex& = GUI::ModelIndex()) const override; @@ -62,5 +62,5 @@ private: GUI::Icon m_section_open_icon; GUI::Icon m_section_icon; GUI::Icon m_page_icon; - mutable HashMap> m_mapped_files; + mutable HashMap> m_mapped_files; }; diff --git a/Applications/Help/main.cpp b/Applications/Help/main.cpp index 31a74791f2..d5a6405fbf 100644 --- a/Applications/Help/main.cpp +++ b/Applications/Help/main.cpp @@ -159,7 +159,7 @@ int main(int argc, char* argv[]) auto source_result = model->page_view(path); if (source_result.is_error()) { - GUI::MessageBox::show(window, strerror(source_result.error()), "Failed to open man page", GUI::MessageBox::Type::Error); + GUI::MessageBox::show(window, source_result.error().string(), "Failed to open man page", GUI::MessageBox::Type::Error); return; } diff --git a/DevTools/HackStudio/Debugger/DisassemblyModel.cpp b/DevTools/HackStudio/Debugger/DisassemblyModel.cpp index fe800df23c..cb7822c60a 100644 --- a/DevTools/HackStudio/Debugger/DisassemblyModel.cpp +++ b/DevTools/HackStudio/Debugger/DisassemblyModel.cpp @@ -51,10 +51,10 @@ DisassemblyModel::DisassemblyModel(const Debug::DebugSession& debug_session, con const ELF::Image* elf = nullptr; if (containing_function.value().address_low >= 0xc0000000) { - auto kernel_file = make("/boot/Kernel"); - if (!kernel_file->is_valid()) + auto file_or_error = MappedFile::map("/boot/Kernel"); + if (file_or_error.is_error()) return; - kernel_elf = make((const u8*)kernel_file->data(), kernel_file->size()); + kernel_elf = make(file_or_error.value()->bytes()); elf = kernel_elf.ptr(); } else { elf = &lib->debug_info->elf(); diff --git a/DevTools/Profiler/DisassemblyModel.cpp b/DevTools/Profiler/DisassemblyModel.cpp index 825d76f4a1..77f74585bd 100644 --- a/DevTools/Profiler/DisassemblyModel.cpp +++ b/DevTools/Profiler/DisassemblyModel.cpp @@ -60,9 +60,10 @@ DisassemblyModel::DisassemblyModel(Profile& profile, ProfileNode& node) FlatPtr base_address = 0; if (m_node.address() >= 0xc0000000) { if (!m_kernel_file) { - m_kernel_file = new MappedFile("/boot/Kernel"); - if (!m_kernel_file->is_valid()) + auto file_or_error = MappedFile::map("/boot/Kernel"); + if (file_or_error.is_error()) return; + m_kernel_file = file_or_error.release_value(); } kernel_elf = make((const u8*)m_kernel_file->data(), m_kernel_file->size()); elf = kernel_elf.ptr(); diff --git a/DevTools/Profiler/DisassemblyModel.h b/DevTools/Profiler/DisassemblyModel.h index cd856d6e21..0d0e643292 100644 --- a/DevTools/Profiler/DisassemblyModel.h +++ b/DevTools/Profiler/DisassemblyModel.h @@ -69,7 +69,7 @@ private: Profile& m_profile; ProfileNode& m_node; - OwnPtr m_kernel_file; + RefPtr m_kernel_file; Vector m_instructions; }; diff --git a/DevTools/Profiler/Profile.cpp b/DevTools/Profiler/Profile.cpp index 11cb49f50e..bf2caa7f08 100644 --- a/DevTools/Profiler/Profile.cpp +++ b/DevTools/Profiler/Profile.cpp @@ -230,10 +230,10 @@ Result, String> Profile::load_from_perfcore_file(const St if (!coredump) return String { "Could not open coredump" }; - MappedFile kernel_elf_file("/boot/Kernel"); + auto file_or_error = MappedFile::map("/boot/Kernel"); OwnPtr kernel_elf; - if (kernel_elf_file.is_valid()) - kernel_elf = make(static_cast(kernel_elf_file.data()), kernel_elf_file.size()); + if (!file_or_error.is_error()) + kernel_elf = make(file_or_error.value()->bytes()); auto events_value = object.get("events"); if (!events_value.is_array()) diff --git a/DevTools/UserspaceEmulator/Emulator.cpp b/DevTools/UserspaceEmulator/Emulator.cpp index 00a88183d5..6ea2b9bff0 100644 --- a/DevTools/UserspaceEmulator/Emulator.cpp +++ b/DevTools/UserspaceEmulator/Emulator.cpp @@ -167,13 +167,14 @@ void Emulator::setup_stack(Vector aux_vector) bool Emulator::load_elf() { - MappedFile mapped_executable(m_executable_path); - if (!mapped_executable.is_valid()) { - reportln("Unable to map {}", m_executable_path); + auto file_or_error = MappedFile::map(m_executable_path); + if (file_or_error.is_error()) { + reportln("Unable to map {}: {}", m_executable_path, file_or_error.error()); return false; } - ELF::Image executable_elf((const u8*)mapped_executable.data(), mapped_executable.size()); + auto elf_image_data = file_or_error.value()->bytes(); + ELF::Image executable_elf(elf_image_data); if (!executable_elf.is_dynamic()) { // FIXME: Support static objects @@ -181,7 +182,7 @@ bool Emulator::load_elf() } String interpreter_path; - if (!ELF::validate_program_headers(*(Elf32_Ehdr*)mapped_executable.data(), mapped_executable.size(), (u8*)mapped_executable.data(), mapped_executable.size(), &interpreter_path)) { + if (!ELF::validate_program_headers(*(const Elf32_Ehdr*)elf_image_data.data(), elf_image_data.size(), (const u8*)elf_image_data.data(), elf_image_data.size(), &interpreter_path)) { reportln("failed to validate ELF file"); return false; } @@ -189,9 +190,10 @@ bool Emulator::load_elf() ASSERT(!interpreter_path.is_null()); dbgln("interpreter: {}", interpreter_path); - auto interpreter_file = make(interpreter_path); - ASSERT(interpreter_file->is_valid()); - ELF::Image interpreter_image((const u8*)interpreter_file->data(), interpreter_file->size()); + auto interpreter_file_or_error = MappedFile::map(interpreter_path); + ASSERT(!interpreter_file_or_error.is_error()); + auto interpreter_image_data = interpreter_file_or_error.value()->bytes(); + ELF::Image interpreter_image(interpreter_image_data); constexpr FlatPtr interpreter_load_offset = 0x08000000; interpreter_image.for_each_program_header([&](const ELF::Image::ProgramHeader& program_header) { @@ -308,12 +310,12 @@ String Emulator::create_backtrace_line(FlatPtr address) lib_path = String::formatted("/usr/lib/{}", lib_path); if (!m_dynamic_library_cache.contains(lib_path)) { - MappedFile mapped_file { lib_path }; - if (!mapped_file.is_valid()) + auto file_or_error = MappedFile::map(lib_path); + if (file_or_error.is_error()) return minimal; - auto debug_info = make(make((const u8*)mapped_file.data(), mapped_file.size())); - m_dynamic_library_cache.set(lib_path, CachedELF { move(mapped_file), move(debug_info) }); + auto debug_info = make(make(file_or_error.value()->bytes())); + m_dynamic_library_cache.set(lib_path, CachedELF { file_or_error.release_value(), move(debug_info) }); } auto it = m_dynamic_library_cache.find(lib_path); @@ -1765,11 +1767,11 @@ int Emulator::virt$beep() bool Emulator::find_malloc_symbols(const MmapRegion& libc_text) { - auto mapped_file = make("/usr/lib/libc.so"); - if (!mapped_file->is_valid()) - return {}; + auto file_or_error = MappedFile::map("/usr/lib/libc.so"); + if (file_or_error.is_error()) + return false; - ELF::Image image((const u8*)mapped_file->data(), mapped_file->size()); + ELF::Image image(file_or_error.value()->bytes()); auto malloc_symbol = image.find_demangled_function("malloc"); auto free_symbol = image.find_demangled_function("free"); auto realloc_symbol = image.find_demangled_function("realloc"); diff --git a/DevTools/UserspaceEmulator/Emulator.h b/DevTools/UserspaceEmulator/Emulator.h index 61191265c1..de8e42f3a8 100644 --- a/DevTools/UserspaceEmulator/Emulator.h +++ b/DevTools/UserspaceEmulator/Emulator.h @@ -207,7 +207,7 @@ private: Optional m_loader_text_size; struct CachedELF { - MappedFile mapped_file; + NonnullRefPtr mapped_file; NonnullOwnPtr debug_info; }; diff --git a/Libraries/LibCoreDump/Backtrace.cpp b/Libraries/LibCoreDump/Backtrace.cpp index 62a7ff9ce1..afa6220447 100644 --- a/Libraries/LibCoreDump/Backtrace.cpp +++ b/Libraries/LibCoreDump/Backtrace.cpp @@ -57,11 +57,12 @@ static const ELFObjectInfo* object_info_for_region(const ELF::Core::MemoryRegion if (!Core::File::exists(path.characters())) return nullptr; - MappedFile object_file(path); - if (!object_file.is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - auto info = make(move(object_file), Debug::DebugInfo { make((const u8*)object_file.data(), object_file.size()) }); + auto image = make(file_or_error.value()->bytes()); + auto info = make(file_or_error.release_value(), Debug::DebugInfo { move(image) }); auto* info_ptr = info.ptr(); s_debug_info_cache.set(path, move(info)); return info_ptr; diff --git a/Libraries/LibCoreDump/Backtrace.h b/Libraries/LibCoreDump/Backtrace.h index b2ded64414..6915b170c2 100644 --- a/Libraries/LibCoreDump/Backtrace.h +++ b/Libraries/LibCoreDump/Backtrace.h @@ -33,13 +33,13 @@ namespace CoreDump { struct ELFObjectInfo { - ELFObjectInfo(MappedFile&& file, Debug::DebugInfo&& debug_info) + ELFObjectInfo(NonnullRefPtr file, Debug::DebugInfo&& debug_info) : file(move(file)) , debug_info(move(debug_info)) { } - MappedFile file; + NonnullRefPtr file; Debug::DebugInfo debug_info; }; diff --git a/Libraries/LibCoreDump/Reader.cpp b/Libraries/LibCoreDump/Reader.cpp index 6646faee48..da7eeabe85 100644 --- a/Libraries/LibCoreDump/Reader.cpp +++ b/Libraries/LibCoreDump/Reader.cpp @@ -35,15 +35,15 @@ namespace CoreDump { OwnPtr Reader::create(const String& path) { - auto mapped_file = make(path); - if (!mapped_file->is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - return make(move(mapped_file)); + return adopt_own(*new Reader(file_or_error.release_value())); } -Reader::Reader(OwnPtr&& coredump_file) +Reader::Reader(NonnullRefPtr coredump_file) : m_coredump_file(move(coredump_file)) - , m_coredump_image((u8*)m_coredump_file->data(), m_coredump_file->size()) + , m_coredump_image(m_coredump_file->bytes()) { size_t index = 0; m_coredump_image.for_each_program_header([this, &index](auto pheader) { @@ -201,11 +201,11 @@ const Reader::LibraryData* Reader::library_containing(FlatPtr address) const } if (!cached_libs.contains(path)) { - auto lib_file = make(path); - if (!lib_file->is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return {}; - auto image = ELF::Image((const u8*)lib_file->data(), lib_file->size()); - cached_libs.set(path, make(name, region->region_start, move(lib_file), move(image))); + auto image = ELF::Image(file_or_error.value()->bytes()); + cached_libs.set(path, make(name, region->region_start, file_or_error.release_value(), move(image))); } auto lib_data = cached_libs.get(path).value(); diff --git a/Libraries/LibCoreDump/Reader.h b/Libraries/LibCoreDump/Reader.h index 6ca90aba14..9ac411a204 100644 --- a/Libraries/LibCoreDump/Reader.h +++ b/Libraries/LibCoreDump/Reader.h @@ -38,13 +38,12 @@ namespace CoreDump { class Reader { AK_MAKE_NONCOPYABLE(Reader); + AK_MAKE_NONMOVABLE(Reader); public: static OwnPtr create(const String&); ~Reader(); - Reader(OwnPtr&&); - const ELF::Core::ProcessInfo& process_info() const; template @@ -61,7 +60,7 @@ public: struct LibraryData { String name; FlatPtr base_address { 0 }; - OwnPtr file; + NonnullRefPtr file; ELF::Image lib_elf; }; const LibraryData* library_containing(FlatPtr address) const; @@ -70,6 +69,8 @@ public: const HashMap metadata() const; private: + Reader(NonnullRefPtr); + class NotesEntryIterator { public: NotesEntryIterator(const u8* notes_data); @@ -85,7 +86,7 @@ private: const u8* start { nullptr }; }; - OwnPtr m_coredump_file; + NonnullRefPtr m_coredump_file; ELF::Image m_coredump_image; ssize_t m_notes_segment_index { -1 }; }; diff --git a/Libraries/LibDebug/DebugSession.cpp b/Libraries/LibDebug/DebugSession.cpp index d77fbbe0cc..d231d243e3 100644 --- a/Libraries/LibDebug/DebugSession.cpp +++ b/Libraries/LibDebug/DebugSession.cpp @@ -42,13 +42,6 @@ DebugSession::DebugSession(pid_t pid, String source_root) { } -MappedFile DebugSession::map_executable_for_process(pid_t pid) -{ - MappedFile executable(String::formatted("/proc/{}/exe", pid)); - ASSERT(executable.is_valid()); - return executable; -} - DebugSession::~DebugSession() { if (m_is_debuggee_dead) @@ -373,13 +366,13 @@ void DebugSession::update_loaded_libs() if (m_loaded_libraries.contains(lib_name)) return IterationDecision::Continue; - MappedFile lib_file(object_path.value()); - if (!lib_file.is_valid()) + auto file_or_error = MappedFile ::map(object_path.value()); + if (file_or_error.is_error()) return IterationDecision::Continue; FlatPtr base_address = entry.as_object().get("address").as_u32(); - auto debug_info = make(make(reinterpret_cast(lib_file.data()), lib_file.size()), m_source_root, base_address); - auto lib = make(lib_name, move(lib_file), move(debug_info), base_address); + auto debug_info = make(make(file_or_error.value()->bytes()), m_source_root, base_address); + auto lib = make(lib_name, file_or_error.release_value(), move(debug_info), base_address); m_loaded_libraries.set(lib_name, move(lib)); return IterationDecision::Continue; diff --git a/Libraries/LibDebug/DebugSession.h b/Libraries/LibDebug/DebugSession.h index bc8bde54f7..f30174a740 100644 --- a/Libraries/LibDebug/DebugSession.h +++ b/Libraries/LibDebug/DebugSession.h @@ -134,11 +134,11 @@ public: struct LoadedLibrary { String name; - MappedFile file; + NonnullRefPtr file; NonnullOwnPtr debug_info; FlatPtr base_address; - LoadedLibrary(const String& name, MappedFile&& file, NonnullOwnPtr&& debug_info, FlatPtr base_address) + LoadedLibrary(const String& name, NonnullRefPtr file, NonnullOwnPtr&& debug_info, FlatPtr base_address) : name(name) , file(move(file)) , debug_info(move(debug_info)) @@ -175,8 +175,6 @@ private: // x86 breakpoint instruction "int3" static constexpr u8 BREAKPOINT_INSTRUCTION = 0xcc; - static MappedFile map_executable_for_process(pid_t); - void update_loaded_libs(); int m_debuggee_pid { -1 }; diff --git a/Libraries/LibELF/Image.cpp b/Libraries/LibELF/Image.cpp index 42c5583750..27bf519353 100644 --- a/Libraries/LibELF/Image.cpp +++ b/Libraries/LibELF/Image.cpp @@ -36,14 +36,19 @@ namespace ELF { -Image::Image(const u8* buffer, size_t size, bool verbose_logging) - : m_buffer(buffer) - , m_size(size) +Image::Image(ReadonlyBytes bytes, bool verbose_logging) + : m_buffer(bytes.data()) + , m_size(bytes.size()) , m_verbose_logging(verbose_logging) { parse(); } +Image::Image(const u8* buffer, size_t size, bool verbose_logging) + : Image(ReadonlyBytes { buffer, size }, verbose_logging) +{ +} + Image::~Image() { } diff --git a/Libraries/LibELF/Image.h b/Libraries/LibELF/Image.h index 7752f7f3e3..3250cf4deb 100644 --- a/Libraries/LibELF/Image.h +++ b/Libraries/LibELF/Image.h @@ -37,7 +37,9 @@ namespace ELF { class Image { public: + explicit Image(ReadonlyBytes, bool verbose_logging = true); explicit Image(const u8*, size_t, bool verbose_logging = true); + ~Image(); void dump() const; bool is_valid() const { return m_valid; } diff --git a/Libraries/LibGUI/FileIconProvider.cpp b/Libraries/LibGUI/FileIconProvider.cpp index 08fc827620..9d2ec26ba0 100644 --- a/Libraries/LibGUI/FileIconProvider.cpp +++ b/Libraries/LibGUI/FileIconProvider.cpp @@ -150,12 +150,14 @@ Icon FileIconProvider::icon_for_executable(const String& path) // If the icon for an app isn't in the cache we attempt to load the file as an ELF image and extract // the serenity_app_icon_* sections which should contain the icons as raw PNG data. In the future it would // be better if the binary signalled the image format being used or we deduced it, e.g. using magic bytes. - auto mapped_file = make(path); - if (!mapped_file->is_valid()) { + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) { app_icon_cache.set(path, s_executable_icon); return s_executable_icon; } + auto& mapped_file = file_or_error.value(); + if (mapped_file->size() < SELFMAG) { app_icon_cache.set(path, s_executable_icon); return s_executable_icon; diff --git a/Libraries/LibGUI/ImageWidget.cpp b/Libraries/LibGUI/ImageWidget.cpp index ca3a736ad6..a77ca3d1b7 100644 --- a/Libraries/LibGUI/ImageWidget.cpp +++ b/Libraries/LibGUI/ImageWidget.cpp @@ -92,10 +92,11 @@ void ImageWidget::animate() void ImageWidget::load_from_file(const StringView& path) { - MappedFile mapped_file(path); - if (!mapped_file.is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return; + auto& mapped_file = *file_or_error.value(); m_image_decoder = Gfx::ImageDecoder::create((const u8*)mapped_file.data(), mapped_file.size()); auto bitmap = m_image_decoder->bitmap(); ASSERT(bitmap); diff --git a/Libraries/LibGfx/BMPLoader.cpp b/Libraries/LibGfx/BMPLoader.cpp index 8f8ccc4d1c..2b178f0257 100644 --- a/Libraries/LibGfx/BMPLoader.cpp +++ b/Libraries/LibGfx/BMPLoader.cpp @@ -179,10 +179,10 @@ static RefPtr load_bmp_impl(const u8*, size_t); RefPtr load_bmp(const StringView& path) { - MappedFile mapped_file(path); - if (!mapped_file.is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - auto bitmap = load_bmp_impl((const u8*)mapped_file.data(), mapped_file.size()); + auto bitmap = load_bmp_impl((const u8*)file_or_error.value()->data(), file_or_error.value()->size()); if (bitmap) bitmap->set_mmap_name(String::formatted("Gfx::Bitmap [{}] - Decoded BMP: {}", bitmap->size(), LexicalPath::canonicalized_path(path))); return bitmap; diff --git a/Libraries/LibGfx/BitmapFont.cpp b/Libraries/LibGfx/BitmapFont.cpp index 26c596cf00..42fefc1530 100644 --- a/Libraries/LibGfx/BitmapFont.cpp +++ b/Libraries/LibGfx/BitmapFont.cpp @@ -27,7 +27,6 @@ #include "BitmapFont.h" #include "Bitmap.h" #include "Emoji.h" -#include #include #include #include @@ -174,15 +173,15 @@ size_t BitmapFont::glyph_count_by_type(FontTypes type) RefPtr BitmapFont::load_from_file(const StringView& path) { - MappedFile mapped_file(path); - if (!mapped_file.is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - auto font = load_from_memory((const u8*)mapped_file.data()); + auto font = load_from_memory((const u8*)file_or_error.value()->data()); if (!font) return nullptr; - font->m_mapped_file = move(mapped_file); + font->m_mapped_file = file_or_error.release_value(); return font; } diff --git a/Libraries/LibGfx/BitmapFont.h b/Libraries/LibGfx/BitmapFont.h index ef62a63da6..c3ca9f4a1d 100644 --- a/Libraries/LibGfx/BitmapFont.h +++ b/Libraries/LibGfx/BitmapFont.h @@ -128,7 +128,7 @@ private: unsigned* m_rows { nullptr }; u8* m_glyph_widths { nullptr }; - MappedFile m_mapped_file; + RefPtr m_mapped_file; u8 m_glyph_width { 0 }; u8 m_glyph_height { 0 }; diff --git a/Libraries/LibGfx/GIFLoader.cpp b/Libraries/LibGfx/GIFLoader.cpp index aad9c6d6ed..d29c5969e3 100644 --- a/Libraries/LibGfx/GIFLoader.cpp +++ b/Libraries/LibGfx/GIFLoader.cpp @@ -107,10 +107,10 @@ struct GIFLoadingContext { RefPtr load_gif(const StringView& path) { - MappedFile mapped_file(path); - if (!mapped_file.is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - GIFImageDecoderPlugin gif_decoder((const u8*)mapped_file.data(), mapped_file.size()); + GIFImageDecoderPlugin gif_decoder((const u8*)file_or_error.value()->data(), file_or_error.value()->size()); auto bitmap = gif_decoder.bitmap(); if (bitmap) bitmap->set_mmap_name(String::formatted("Gfx::Bitmap [{}] - Decoded GIF: {}", bitmap->size(), LexicalPath::canonicalized_path(path))); diff --git a/Libraries/LibGfx/ICOLoader.cpp b/Libraries/LibGfx/ICOLoader.cpp index 6fc0f790e5..031f4e657e 100644 --- a/Libraries/LibGfx/ICOLoader.cpp +++ b/Libraries/LibGfx/ICOLoader.cpp @@ -116,10 +116,10 @@ struct ICOLoadingContext { RefPtr load_ico(const StringView& path) { - MappedFile mapped_file(path); - if (!mapped_file.is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - ICOImageDecoderPlugin decoder((const u8*)mapped_file.data(), mapped_file.size()); + ICOImageDecoderPlugin decoder((const u8*)file_or_error.value()->data(), file_or_error.value()->size()); auto bitmap = decoder.bitmap(); if (bitmap) bitmap->set_mmap_name(String::formatted("Gfx::Bitmap [{}] - Decoded ICO: {}", bitmap->size(), LexicalPath::canonicalized_path(path))); diff --git a/Libraries/LibGfx/JPGLoader.cpp b/Libraries/LibGfx/JPGLoader.cpp index 124dadae4d..ded73a59fd 100644 --- a/Libraries/LibGfx/JPGLoader.cpp +++ b/Libraries/LibGfx/JPGLoader.cpp @@ -1320,12 +1320,10 @@ static RefPtr load_jpg_impl(const u8* data, size_t data_size) RefPtr load_jpg(const StringView& path) { - MappedFile mapped_file(path); - if (!mapped_file.is_valid()) { + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - } - - auto bitmap = load_jpg_impl((const u8*)mapped_file.data(), mapped_file.size()); + auto bitmap = load_jpg_impl((const u8*)file_or_error.value()->data(), file_or_error.value()->size()); if (bitmap) bitmap->set_mmap_name(String::formatted("Gfx::Bitmap [{}] - Decoded JPG: {}", bitmap->size(), LexicalPath::canonicalized_path(path))); return bitmap; diff --git a/Libraries/LibGfx/PNGLoader.cpp b/Libraries/LibGfx/PNGLoader.cpp index 687d0231c7..48405f8bea 100644 --- a/Libraries/LibGfx/PNGLoader.cpp +++ b/Libraries/LibGfx/PNGLoader.cpp @@ -193,10 +193,10 @@ static bool process_chunk(Streamer&, PNGLoadingContext& context); RefPtr load_png(const StringView& path) { - MappedFile mapped_file(path); - if (!mapped_file.is_valid()) + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - auto bitmap = load_png_impl((const u8*)mapped_file.data(), mapped_file.size()); + auto bitmap = load_png_impl((const u8*)file_or_error.value()->data(), file_or_error.value()->size()); if (bitmap) bitmap->set_mmap_name(String::formatted("Gfx::Bitmap [{}] - Decoded PNG: {}", bitmap->size(), LexicalPath::canonicalized_path(path))); return bitmap; diff --git a/Libraries/LibGfx/PortableImageLoaderCommon.h b/Libraries/LibGfx/PortableImageLoaderCommon.h index b5b1c5e91f..5f3c51679c 100644 --- a/Libraries/LibGfx/PortableImageLoaderCommon.h +++ b/Libraries/LibGfx/PortableImageLoaderCommon.h @@ -294,12 +294,10 @@ static RefPtr load_impl(const u8* data, size_t data_size) template static RefPtr load(const StringView& path) { - MappedFile mapped_file(path); - if (!mapped_file.is_valid()) { + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) return nullptr; - } - - auto bitmap = load_impl((const u8*)mapped_file.data(), mapped_file.size()); + auto bitmap = load_impl((const u8*)file_or_error.value()->data(), file_or_error.value()->size()); if (bitmap) bitmap->set_mmap_name(String::formatted("Gfx::Bitmap [{}] - Decoded {}: {}", bitmap->size(), diff --git a/Libraries/LibPCIDB/Database.cpp b/Libraries/LibPCIDB/Database.cpp index 83c65abb6b..8afc5a2f2d 100644 --- a/Libraries/LibPCIDB/Database.cpp +++ b/Libraries/LibPCIDB/Database.cpp @@ -34,7 +34,10 @@ namespace PCIDB { RefPtr Database::open(const StringView& file_name) { - auto res = adopt(*new Database(file_name)); + auto file_or_error = MappedFile::map(file_name); + if (file_or_error.is_error()) + return nullptr; + auto res = adopt(*new Database(file_or_error.release_value())); if (res->init() != 0) return nullptr; return res; @@ -131,10 +134,7 @@ int Database::init() if (m_ready) return 0; - if (!m_file.is_valid()) - return -1; - - m_view = StringView((const char*)m_file.data(), m_file.size()); + m_view = StringView { m_file->bytes() }; ParseMode mode = ParseMode::UnknownMode; diff --git a/Libraries/LibPCIDB/Database.h b/Libraries/LibPCIDB/Database.h index 62817c4c7d..dbbe8c9482 100644 --- a/Libraries/LibPCIDB/Database.h +++ b/Libraries/LibPCIDB/Database.h @@ -83,8 +83,10 @@ public: const StringView get_programming_interface(u8 class_id, u8 subclass_id, u8 programming_interface_id) const; private: - Database(const StringView& file_name) - : m_file(file_name) {}; + explicit Database(NonnullRefPtr file) + : m_file(move(file)) + { + } int init(); @@ -94,7 +96,7 @@ private: ClassMode, }; - MappedFile m_file {}; + NonnullRefPtr m_file; StringView m_view {}; HashMap> m_vendors; HashMap> m_classes; diff --git a/Services/WebServer/Client.cpp b/Services/WebServer/Client.cpp index deb582e013..84ba323d64 100644 --- a/Services/WebServer/Client.cpp +++ b/Services/WebServer/Client.cpp @@ -162,8 +162,9 @@ static String folder_image_data() { static String cache; if (cache.is_empty()) { - MappedFile image("/res/icons/16x16/filetype-folder.png"); - cache = encode_base64({ image.data(), image.size() }); + auto file_or_error = MappedFile::map("/res/icons/16x16/filetype-folder.png"); + ASSERT(!file_or_error.is_error()); + cache = encode_base64(file_or_error.value()->bytes()); } return cache; } @@ -172,8 +173,9 @@ static String file_image_data() { static String cache; if (cache.is_empty()) { - MappedFile image("/res/icons/16x16/filetype-unknown.png"); - cache = encode_base64({ image.data(), image.size() }); + auto file_or_error = MappedFile::map("/res/icons/16x16/filetype-unknown.png"); + ASSERT(!file_or_error.is_error()); + cache = encode_base64(file_or_error.value()->bytes()); } return cache; } diff --git a/Userland/disasm.cpp b/Userland/disasm.cpp index 4189499000..781caf0796 100644 --- a/Userland/disasm.cpp +++ b/Userland/disasm.cpp @@ -48,12 +48,14 @@ int main(int argc, char** argv) args_parser.add_positional_argument(path, "Path to i386 binary file", "path"); args_parser.parse(argc, argv); - MappedFile file(path); - if (!file.is_valid()) { - // Already printed some error message. + auto file_or_error = MappedFile::map(path); + if (file_or_error.is_error()) { + warnln("Could not map file: {}", file_or_error.error().string()); return 1; } + auto& file = *file_or_error.value(); + struct Symbol { size_t value; size_t size; diff --git a/Userland/functrace.cpp b/Userland/functrace.cpp index 02c4cc4b6a..763e7a4a35 100644 --- a/Userland/functrace.cpp +++ b/Userland/functrace.cpp @@ -90,7 +90,7 @@ static NonnullOwnPtr> instrument_code() if (section.name() != ".text") return IterationDecision::Continue; - X86::SimpleInstructionStream stream((const u8*)((u32)lib.file.data() + section.offset()), section.size()); + X86::SimpleInstructionStream stream((const u8*)((u32)lib.file->data() + section.offset()), section.size()); X86::Disassembler disassembler(stream); for (;;) { auto offset = stream.offset(); diff --git a/Userland/readelf.cpp b/Userland/readelf.cpp index ff0ccd613a..08bd2e6607 100644 --- a/Userland/readelf.cpp +++ b/Userland/readelf.cpp @@ -284,14 +284,15 @@ int main(int argc, char** argv) display_symbol_table = true; } - MappedFile mapped_executable(path); + auto file_or_error = MappedFile::map(path); - if (!mapped_executable.is_valid()) { - fprintf(stderr, "Unable to map file %s\n", path); + if (file_or_error.is_error()) { + warnln("Unable to map file {}: {}", path, file_or_error.error()); return -1; } - ELF::Image executable_elf((const u8*)mapped_executable.data(), mapped_executable.size()); + auto elf_image_data = file_or_error.value()->bytes(); + ELF::Image executable_elf(elf_image_data); if (!executable_elf.is_valid()) { fprintf(stderr, "File is not a valid ELF object\n"); @@ -300,7 +301,7 @@ int main(int argc, char** argv) String interpreter_path; - if (!ELF::validate_program_headers(*(Elf32_Ehdr*)mapped_executable.data(), mapped_executable.size(), (u8*)mapped_executable.data(), mapped_executable.size(), &interpreter_path)) { + if (!ELF::validate_program_headers(*(const Elf32_Ehdr*)elf_image_data.data(), elf_image_data.size(), (const u8*)elf_image_data.data(), elf_image_data.size(), &interpreter_path)) { fprintf(stderr, "Invalid ELF headers\n"); return -1; } @@ -309,14 +310,14 @@ int main(int argc, char** argv) fprintf(stderr, "Warning: Dynamic ELF object has no interpreter path\n"); } - ELF::Image interpreter_image((const u8*)mapped_executable.data(), mapped_executable.size()); + ELF::Image interpreter_image(elf_image_data); if (!interpreter_image.is_valid()) { fprintf(stderr, "ELF image is invalid\n"); return -1; } - auto header = *reinterpret_cast(mapped_executable.data()); + auto& header = *reinterpret_cast(elf_image_data.data()); if (display_elf_header) { printf("ELF header:\n"); diff --git a/Userland/unzip.cpp b/Userland/unzip.cpp index 2617c353de..d21228c822 100644 --- a/Userland/unzip.cpp +++ b/Userland/unzip.cpp @@ -181,9 +181,12 @@ int main(int argc, char** argv) return 1; } - MappedFile mapped_file { zip_file_path }; - if (!mapped_file.is_valid()) + auto file_or_error = MappedFile ::map(zip_file_path); + if (file_or_error.is_error()) { + warnln("Failed to open {}: {}", zip_file_path, file_or_error.error()); return 1; + } + auto& mapped_file = *file_or_error.value(); printf("Archive: %s\n", zip_file_path.characters());