Przeglądaj źródła

AK: Use ErrorOr<T> for MappedFile factories

Replace Result<T, E> with ErrorOr<T> and propagate the error to callers.
Andreas Kling 3 lat temu
rodzic
commit
0f5477c721

+ 8 - 11
AK/MappedFile.cpp

@@ -15,36 +15,33 @@
 
 namespace AK {
 
-Result<NonnullRefPtr<MappedFile>, OSError> MappedFile::map(String const& path)
+ErrorOr<NonnullRefPtr<MappedFile>> MappedFile::map(String const& path)
 {
     int fd = open(path.characters(), O_RDONLY | O_CLOEXEC, 0);
     if (fd < 0)
-        return OSError(errno);
+        return Error::from_errno(errno);
 
     return map_from_fd_and_close(fd, path);
 }
 
-Result<NonnullRefPtr<MappedFile>, OSError> MappedFile::map_from_fd_and_close(int fd, [[maybe_unused]] String const& path)
+ErrorOr<NonnullRefPtr<MappedFile>> MappedFile::map_from_fd_and_close(int fd, [[maybe_unused]] String const& path)
 {
-    if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) {
-        return OSError(errno);
-    }
+    if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0)
+        return Error::from_errno(errno);
 
     ScopeGuard fd_close_guard = [fd] {
         close(fd);
     };
 
     struct stat st;
-    if (fstat(fd, &st) < 0) {
-        auto saved_errno = errno;
-        return OSError(saved_errno);
-    }
+    if (fstat(fd, &st) < 0)
+        return Error::from_errno(errno);
 
     auto size = st.st_size;
     auto* ptr = mmap(nullptr, size, PROT_READ, MAP_SHARED, fd, 0);
 
     if (ptr == MAP_FAILED)
-        return OSError(errno);
+        return Error::from_errno(errno);
 
 #ifdef __serenity__
     if (set_mmap_name(ptr, size, path.characters()) < 0) {

+ 3 - 3
AK/MappedFile.h

@@ -6,9 +6,9 @@
 
 #pragma once
 
+#include <AK/Error.h>
 #include <AK/Noncopyable.h>
 #include <AK/NonnullRefPtr.h>
-#include <AK/OSError.h>
 #include <AK/RefCounted.h>
 #include <AK/Result.h>
 
@@ -19,8 +19,8 @@ class MappedFile : public RefCounted<MappedFile> {
     AK_MAKE_NONMOVABLE(MappedFile);
 
 public:
-    static Result<NonnullRefPtr<MappedFile>, OSError> map(String const& path);
-    static Result<NonnullRefPtr<MappedFile>, OSError> map_from_fd_and_close(int fd, String const& path);
+    static ErrorOr<NonnullRefPtr<MappedFile>> map(String const& path);
+    static ErrorOr<NonnullRefPtr<MappedFile>> map_from_fd_and_close(int fd, String const& path);
     ~MappedFile();
 
     void* data() { return m_data; }

+ 5 - 6
Userland/Applications/Help/ManualModel.cpp

@@ -9,6 +9,7 @@
 #include "ManualPageNode.h"
 #include "ManualSectionNode.h"
 #include <AK/ByteBuffer.h>
+#include <AK/Try.h>
 #include <LibCore/File.h>
 #include <LibGUI/FilteringProxyModel.h>
 
@@ -59,7 +60,7 @@ String ManualModel::page_path(const GUI::ModelIndex& index) const
     return page->path();
 }
 
-Result<StringView, OSError> ManualModel::page_view(const String& path) const
+ErrorOr<StringView> ManualModel::page_view(String const& path) const
 {
     if (path.is_empty())
         return StringView {};
@@ -71,12 +72,10 @@ Result<StringView, OSError> ManualModel::page_view(const String& path) const
             return StringView { mapped_file.value()->bytes() };
     }
 
-    auto file_or_error = MappedFile::map(path);
-    if (file_or_error.is_error())
-        return file_or_error.error();
+    auto file = TRY(MappedFile::map(path));
 
-    StringView view { file_or_error.value()->bytes() };
-    m_mapped_files.set(path, file_or_error.release_value());
+    StringView view { file->bytes() };
+    m_mapped_files.set(path, move(file));
     return view;
 }
 

+ 1 - 1
Userland/Applications/Help/ManualModel.h

@@ -25,7 +25,7 @@ public:
 
     String page_path(const GUI::ModelIndex&) const;
     String page_and_section(const GUI::ModelIndex&) const;
-    Result<StringView, OSError> page_view(const String& path) const;
+    ErrorOr<StringView> page_view(String const& path) const;
 
     void update_section_node_on_toggle(const GUI::ModelIndex&, const bool);
     virtual int row_count(const GUI::ModelIndex& = GUI::ModelIndex()) const override;

+ 1 - 1
Userland/Applications/Help/main.cpp

@@ -139,7 +139,7 @@ int main(int argc, char* argv[])
 
         auto source_result = model->page_view(path);
         if (source_result.is_error()) {
-            GUI::MessageBox::show(window, source_result.error().string(), "Failed to open man page", GUI::MessageBox::Type::Error);
+            GUI::MessageBox::show(window, String::formatted("{}", source_result.error()), "Failed to open man page", GUI::MessageBox::Type::Error);
             return;
         }
 

+ 1 - 1
Userland/Applications/PixelPaint/ProjectLoader.cpp

@@ -31,7 +31,7 @@ Result<void, String> ProjectLoader::try_load_from_fd_and_close(int fd, StringVie
 
         auto file_or_error = MappedFile::map_from_fd_and_close(fd, path);
         if (file_or_error.is_error())
-            return String::formatted("Unable to mmap file {}", file_or_error.error().string());
+            return String::formatted("Unable to mmap file {}", file_or_error.error());
 
         auto& mapped_file = *file_or_error.value();
         // FIXME: Find a way to avoid the memory copy here.

+ 28 - 34
Userland/Libraries/LibGfx/TrueTypeFont/Font.cpp

@@ -7,6 +7,7 @@
 
 #include <AK/Checked.h>
 #include <AK/MappedFile.h>
+#include <AK/Try.h>
 #include <AK/Utf32View.h>
 #include <AK/Utf8View.h>
 #include <LibCore/File.h>
@@ -224,52 +225,45 @@ GlyphHorizontalMetrics Hmtx::get_glyph_horizontal_metrics(u32 glyph_id) const
     };
 }
 
-Result<NonnullRefPtr<Font>, String> Font::try_load_from_file(String path, unsigned index)
+ErrorOr<NonnullRefPtr<Font>> Font::try_load_from_file(String path, unsigned index)
 {
-    auto file_or_error = MappedFile::map(path);
-    if (file_or_error.is_error())
-        return String { file_or_error.error().string() };
-
-    auto& file = *file_or_error.value();
-    auto result = try_load_from_externally_owned_memory(file.bytes(), index);
-    if (result.is_error())
-        return result.error();
-    auto& font = *result.value();
-    font.m_mapped_file = file_or_error.release_value();
-    return result;
+    auto file = TRY(MappedFile::map(path));
+    auto font = TRY(try_load_from_externally_owned_memory(file->bytes(), index));
+    font->m_mapped_file = move(file);
+    return font;
 }
 
-Result<NonnullRefPtr<Font>, String> Font::try_load_from_externally_owned_memory(ReadonlyBytes buffer, unsigned index)
+ErrorOr<NonnullRefPtr<Font>> Font::try_load_from_externally_owned_memory(ReadonlyBytes buffer, unsigned index)
 {
     if (buffer.size() < 4)
-        return String { "Font file too small" };
+        return Error::from_string_literal("Font file too small"sv);
 
     u32 tag = be_u32(buffer.data());
     if (tag == tag_from_str("ttcf")) {
         // It's a font collection
         if (buffer.size() < (u32)Sizes::TTCHeaderV1 + sizeof(u32) * (index + 1))
-            return String { "Font file too small" };
+            return Error::from_string_literal("Font file too small"sv);
 
         u32 offset = be_u32(buffer.offset_pointer((u32)Sizes::TTCHeaderV1 + sizeof(u32) * index));
-        return try_load_from_offset(move(buffer), offset);
+        return try_load_from_offset(buffer, offset);
     }
     if (tag == tag_from_str("OTTO"))
-        return String { "CFF fonts not supported yet" };
+        return Error::from_string_literal("CFF fonts not supported yet"sv);
 
     if (tag != 0x00010000)
-        return String { "Not a valid font" };
+        return Error::from_string_literal("Not a valid font"sv);
 
-    return try_load_from_offset(move(buffer), 0);
+    return try_load_from_offset(buffer, 0);
 }
 
 // FIXME: "loca" and "glyf" are not available for CFF fonts.
-Result<NonnullRefPtr<Font>, String> Font::try_load_from_offset(ReadonlyBytes buffer, u32 offset)
+ErrorOr<NonnullRefPtr<Font>> Font::try_load_from_offset(ReadonlyBytes buffer, u32 offset)
 {
     if (Checked<u32>::addition_would_overflow(offset, (u32)Sizes::OffsetTable))
-        return String { "Invalid offset in font header" };
+        return Error::from_string_literal("Invalid offset in font header"sv);
 
     if (buffer.size() < offset + (u32)Sizes::OffsetTable)
-        return String { "Font file too small" };
+        return Error::from_string_literal("Font file too small"sv);
 
     Optional<ReadonlyBytes> opt_head_slice = {};
     Optional<ReadonlyBytes> opt_name_slice = {};
@@ -292,7 +286,7 @@ Result<NonnullRefPtr<Font>, String> Font::try_load_from_offset(ReadonlyBytes buf
 
     auto num_tables = be_u16(buffer.offset_pointer(offset + (u32)Offsets::NumTables));
     if (buffer.size() < offset + (u32)Sizes::OffsetTable + num_tables * (u32)Sizes::TableRecord)
-        return String { "Font file too small" };
+        return Error::from_string_literal("Font file too small"sv);
 
     for (auto i = 0; i < num_tables; i++) {
         u32 record_offset = offset + (u32)Sizes::OffsetTable + i * (u32)Sizes::TableRecord;
@@ -301,10 +295,10 @@ Result<NonnullRefPtr<Font>, String> Font::try_load_from_offset(ReadonlyBytes buf
         u32 table_length = be_u32(buffer.offset_pointer(record_offset + (u32)Offsets::TableRecord_Length));
 
         if (Checked<u32>::addition_would_overflow(table_offset, table_length))
-            return String { "Invalid table offset/length in font." };
+            return Error::from_string_literal("Invalid table offset/length in font."sv);
 
         if (buffer.size() < table_offset + table_length)
-            return String { "Font file too small" };
+            return Error::from_string_literal("Font file too small"sv);
 
         auto buffer_here = ReadonlyBytes(buffer.offset_pointer(table_offset), table_length);
 
@@ -331,39 +325,39 @@ Result<NonnullRefPtr<Font>, String> Font::try_load_from_offset(ReadonlyBytes buf
     }
 
     if (!opt_head_slice.has_value() || !(opt_head = Head::from_slice(opt_head_slice.value())).has_value())
-        return String { "Could not load Head" };
+        return Error::from_string_literal("Could not load Head"sv);
     auto head = opt_head.value();
 
     if (!opt_name_slice.has_value() || !(opt_name = Name::from_slice(opt_name_slice.value())).has_value())
-        return String { "Could not load Name" };
+        return Error::from_string_literal("Could not load Name"sv);
     auto name = opt_name.value();
 
     if (!opt_hhea_slice.has_value() || !(opt_hhea = Hhea::from_slice(opt_hhea_slice.value())).has_value())
-        return String { "Could not load Hhea" };
+        return Error::from_string_literal("Could not load Hhea"sv);
     auto hhea = opt_hhea.value();
 
     if (!opt_maxp_slice.has_value() || !(opt_maxp = Maxp::from_slice(opt_maxp_slice.value())).has_value())
-        return String { "Could not load Maxp" };
+        return Error::from_string_literal("Could not load Maxp"sv);
     auto maxp = opt_maxp.value();
 
     if (!opt_hmtx_slice.has_value() || !(opt_hmtx = Hmtx::from_slice(opt_hmtx_slice.value(), maxp.num_glyphs(), hhea.number_of_h_metrics())).has_value())
-        return String { "Could not load Hmtx" };
+        return Error::from_string_literal("Could not load Hmtx"sv);
     auto hmtx = opt_hmtx.value();
 
     if (!opt_cmap_slice.has_value() || !(opt_cmap = Cmap::from_slice(opt_cmap_slice.value())).has_value())
-        return String { "Could not load Cmap" };
+        return Error::from_string_literal("Could not load Cmap"sv);
     auto cmap = opt_cmap.value();
 
     if (!opt_loca_slice.has_value() || !(opt_loca = Loca::from_slice(opt_loca_slice.value(), maxp.num_glyphs(), head.index_to_loc_format())).has_value())
-        return String { "Could not load Loca" };
+        return Error::from_string_literal("Could not load Loca"sv);
     auto loca = opt_loca.value();
 
     if (!opt_glyf_slice.has_value())
-        return String { "Could not load Glyf" };
+        return Error::from_string_literal("Could not load Glyf"sv);
     auto glyf = Glyf(opt_glyf_slice.value());
 
     if (!opt_os2_slice.has_value())
-        return String { "Could not load OS/2" };
+        return Error::from_string_literal("Could not load OS/2"sv);
     auto os2 = OS2(opt_os2_slice.value());
 
     // Select cmap table. FIXME: Do this better. Right now, just looks for platform "Windows"

+ 3 - 3
Userland/Libraries/LibGfx/TrueTypeFont/Font.h

@@ -45,8 +45,8 @@ class Font : public RefCounted<Font> {
     AK_MAKE_NONCOPYABLE(Font);
 
 public:
-    static Result<NonnullRefPtr<Font>, String> try_load_from_file(String path, unsigned index = 0);
-    static Result<NonnullRefPtr<Font>, String> try_load_from_externally_owned_memory(ReadonlyBytes bytes, unsigned index = 0);
+    static ErrorOr<NonnullRefPtr<Font>> try_load_from_file(String path, unsigned index = 0);
+    static ErrorOr<NonnullRefPtr<Font>> try_load_from_externally_owned_memory(ReadonlyBytes bytes, unsigned index = 0);
 
     ScaledFontMetrics metrics(float x_scale, float y_scale) const;
     ScaledGlyphMetrics glyph_metrics(u32 glyph_id, float x_scale, float y_scale) const;
@@ -71,7 +71,7 @@ private:
         TableRecord = 16,
     };
 
-    static Result<NonnullRefPtr<Font>, String> try_load_from_offset(ReadonlyBytes, unsigned index = 0);
+    static ErrorOr<NonnullRefPtr<Font>> try_load_from_offset(ReadonlyBytes, unsigned index = 0);
 
     Font(ReadonlyBytes bytes, Head&& head, Name&& name, Hhea&& hhea, Maxp&& maxp, Hmtx&& hmtx, Cmap&& cmap, Loca&& loca, Glyf&& glyf, OS2&& os2)
         : m_buffer(move(bytes))

+ 1 - 1
Userland/Libraries/LibSymbolication/Symbolication.cpp

@@ -83,7 +83,7 @@ Optional<Symbol> symbolicate(String const& path, FlatPtr address, IncludeSourceP
     if (!s_cache.contains(full_path)) {
         auto mapped_file = MappedFile::map(full_path);
         if (mapped_file.is_error()) {
-            dbgln("Failed to map {}: {}", full_path, mapped_file.error().string());
+            dbgln("Failed to map {}: {}", full_path, mapped_file.error());
             s_cache.set(full_path, {});
             return {};
         }

+ 1 - 1
Userland/Services/CrashDaemon/main.cpp

@@ -74,7 +74,7 @@ int main()
 
         auto file_or_error = MappedFile::map(coredump_path);
         if (file_or_error.is_error()) {
-            dbgln("Unable to map coredump {}: {}", coredump_path, file_or_error.error().string());
+            dbgln("Unable to map coredump {}: {}", coredump_path, file_or_error.error());
             continue;
         }
 

+ 1 - 1
Userland/Utilities/disasm.cpp

@@ -28,7 +28,7 @@ int main(int argc, char** argv)
 
     auto file_or_error = MappedFile::map(path);
     if (file_or_error.is_error()) {
-        warnln("Could not map file: {}", file_or_error.error().string());
+        warnln("Could not map file: {}", file_or_error.error());
         return 1;
     }
 

+ 1 - 1
Userland/Utilities/fdtdump.cpp

@@ -26,7 +26,7 @@ int main(int argc, char* argv[])
     // FIXME: Figure out how to do this sanely from stdin
     auto maybe_file = MappedFile::map(filename);
     if (maybe_file.is_error()) {
-        warnln("Unable to dump device tree from file {}: {}", filename, maybe_file.error().string());
+        warnln("Unable to dump device tree from file {}: {}", filename, maybe_file.error());
         return 1;
     }
     auto file = maybe_file.release_value();