AK: Rework error types to store kind as an enum

This commit makes windows errors store the code instead of being a
formatted string literal. This will allow comparing against the error,
and checking what it is. The formatting is now done in the formatter,
and moved to an implementation file to avoid polluting headers with
windows.h. The kind of the error is now determined by an enum Kind which
will allow matching against the error kind.

Co-Authored-By: Andrew Kaster <andrew@ladybird.org>

AK/Error.cpp

@@ -23,33 +23,9 @@ Error Error::from_string_view_or_print_error_and_return_errno(StringView string_
 }
 
 #ifdef AK_OS_WINDOWS
-Error Error::from_windows_error(u64 code)
+Error Error::from_windows_error(u32 windows_error)
 {
-    thread_local HashMap<u64, ByteString> s_windows_errors;
-
-    auto string = s_windows_errors.get(code);
-    if (string.has_value())
-        return Error::from_string_view(string->view());
-
-    char* message = nullptr;
-    auto size = FormatMessage(
-        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
-        nullptr,
-        static_cast<DWORD>(code),
-        MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
-        reinterpret_cast<LPSTR>(&message),
-        0,
-        nullptr);
-
-    if (size == 0) {
-        static char buffer[128];
-        (void)snprintf(buffer, _countof(buffer), "Error 0x%08lX while getting text of error 0x%08llX", GetLastError(), code);
-        return Error::from_string_view({ buffer, _countof(buffer) });
-    }
-
-    auto& string_in_map = s_windows_errors.ensure(code, [message, size] { return ByteString { message, size }; });
-    LocalFree(message);
-    return Error::from_string_view(string_in_map.view());
+    return Error(static_cast<int>(windows_error), Error::Kind::Windows);
 }
 
 // This can be used both for generic Windows errors and for winsock errors because WSAGetLastError is forwarded to GetLastError.

AK/Error.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/Assertions.h>
 #include <AK/StringView.h>
 #include <AK/Variant.h>
 #include <errno.h>
@@ -17,6 +18,13 @@ public:
     ALWAYS_INLINE Error(Error&&) = default;
     ALWAYS_INLINE Error& operator=(Error&&) = default;
 
+    enum class Kind : u8 {
+        Errno,
+        Syscall,
+        Windows,
+        StringLiteral
+    };
+
     static Error from_errno(int code)
     {
         VERIFY(code != 0);
@@ -24,7 +32,7 @@ public:
     }
 
 #ifdef AK_OS_WINDOWS
-    static Error from_windows_error(u64 code);
+    static Error from_windows_error(u32 windows_error);
     static Error from_windows_error();
 #endif
 
@@ -75,39 +83,53 @@ public:
 
     bool operator==(Error const& other) const
     {
-        return m_code == other.m_code && m_string_literal == other.m_string_literal && m_syscall == other.m_syscall;
+        return m_code == other.m_code && m_string_literal == other.m_string_literal && m_kind == other.m_kind;
     }
 
     int code() const { return m_code; }
     bool is_errno() const
     {
-        return m_code != 0;
+        return m_kind == Kind::Errno;
     }
     bool is_syscall() const
     {
-        return m_syscall;
+        return m_kind == Kind::Syscall;
+    }
+    bool is_windows_error() const
+    {
+        return m_kind == Kind::Windows;
+    }
+    bool is_string_literal() const
+    {
+        return m_kind == Kind::StringLiteral;
     }
     StringView string_literal() const
     {
         return m_string_literal;
     }
+    Kind kind() const
+    {
+        return m_kind;
+    }
 
 protected:
-    Error(int code)
+    Error(int code, Kind kind = Kind::Errno)
         : m_code(code)
+        , m_kind(kind)
     {
     }
 
 private:
     Error(StringView string_literal)
         : m_string_literal(string_literal)
+        , m_kind(Kind::StringLiteral)
     {
     }
 
     Error(StringView syscall_name, int rc)
         : m_string_literal(syscall_name)
         , m_code(-rc)
-        , m_syscall(true)
+        , m_kind(Kind::Syscall)
     {
     }
 
@@ -118,7 +140,7 @@ private:
 
     int m_code { 0 };
 
-    bool m_syscall { false };
+    Kind m_kind {};
 };
 
 template<typename T, typename E>

AK/Format.cpp

@@ -4,9 +4,13 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <AK/Assertions.h>
+#include <AK/ByteString.h>
 #include <AK/CharacterTypes.h>
+#include <AK/Error.h>
 #include <AK/Format.h>
 #include <AK/GenericLexer.h>
+#include <AK/HashMap.h>
 #include <AK/IntegralMath.h>
 #include <AK/LexicalPath.h>
 #include <AK/String.h>
@@ -1087,6 +1091,56 @@ void vout(FILE* file, StringView fmtstr, TypeErasedFormatParams& params, bool ne
         dbgln("vout() failed ({} written out of {}), error was {} ({})", retval, string.length(), error, strerror(error));
     }
 }
+#if defined(AK_OS_WINDOWS)
+ErrorOr<void> Formatter<Error>::format_windows_error(FormatBuilder& builder, Error const& error)
+{
+    thread_local HashMap<u32, ByteString> windows_errors;
+
+    int code = error.code();
+    Optional<ByteString&> string = windows_errors.get(static_cast<u32>(code));
+    if (string.has_value()) {
+        return Formatter<StringView>::format(builder, string->view());
+    }
+
+    TCHAR* message = nullptr;
+    u32 size = FormatMessage(
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+        nullptr,
+        static_cast<DWORD>(code),
+        MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+        message,
+        0,
+        nullptr);
+    if (size == 0) {
+        auto format_error = GetLastError();
+        return Formatter<FormatString>::format(builder, "Error {:08x} when formatting code {:08x}"sv, format_error, code);
+    }
+
+    auto& string_in_map = windows_errors.ensure(code, [message, size] { return ByteString { message, size }; });
+    LocalFree(message);
+    return Formatter<StringView>::format(builder, string_in_map.view());
+}
+#else
+ErrorOr<void> Formatter<Error>::format_windows_error(FormatBuilder&, Error const&)
+{
+    VERIFY_NOT_REACHED();
+}
+#endif
+
+ErrorOr<void> Formatter<Error>::format(FormatBuilder& builder, Error const& error)
+{
+    switch (error.kind()) {
+    case Error::Kind::Syscall:
+        return Formatter<FormatString>::format(builder, "{}: {} (errno={})"sv, error.string_literal(), strerror(error.code()), error.code());
+    case Error::Kind::Errno:
+        return Formatter<FormatString>::format(builder, "{} (errno={})"sv, strerror(error.code()), error.code());
+    case Error::Kind::Windows:
+        return Formatter<Error>::format_windows_error(builder, error);
+    case Error::Kind::StringLiteral:
+        return Formatter<FormatString>::format(builder, "{}"sv, error.string_literal());
+    }
+    VERIFY_NOT_REACHED();
+}
 
 #ifdef AK_OS_ANDROID
 static char const* s_log_tag_name = "Serenity";

AK/Format.h

@@ -737,15 +737,10 @@ struct Formatter<FormatString> : Formatter<StringView> {
 
 template<>
 struct Formatter<Error> : Formatter<FormatString> {
-    ErrorOr<void> format(FormatBuilder& builder, Error const& error)
-    {
-        if (error.is_syscall())
-            return Formatter<FormatString>::format(builder, "{}: {} (errno={})"sv, error.string_literal(), strerror(error.code()), error.code());
-        if (error.is_errno())
-            return Formatter<FormatString>::format(builder, "{} (errno={})"sv, strerror(error.code()), error.code());
+    ErrorOr<void> format(FormatBuilder& builder, Error const& error);
 
-        return Formatter<FormatString>::format(builder, "{}"sv, error.string_literal());
-    }
+private:
+    ErrorOr<void> format_windows_error(FormatBuilder& builder, Error const& error);
 };
 
 template<typename T, typename ErrorType>