From 79b652c74e25301c39193b5ca8a1146682e3406f Mon Sep 17 00:00:00 2001 From: R-Goc Date: Fri, 4 Apr 2025 09:08:13 +0200 Subject: [PATCH] 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 --- AK/Error.cpp | 28 ++------------------------ AK/Error.h | 36 +++++++++++++++++++++++++++------- AK/Format.cpp | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++ AK/Format.h | 11 +++-------- 4 files changed, 88 insertions(+), 41 deletions(-) diff --git a/AK/Error.cpp b/AK/Error.cpp index 8fec89ddd62..dcff18243e6 100644 --- a/AK/Error.cpp +++ b/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 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(code), - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), - reinterpret_cast(&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(windows_error), Error::Kind::Windows); } // This can be used both for generic Windows errors and for winsock errors because WSAGetLastError is forwarded to GetLastError. diff --git a/AK/Error.h b/AK/Error.h index 84585ababfb..4b571e2474f 100644 --- a/AK/Error.h +++ b/AK/Error.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -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 diff --git a/AK/Format.cpp b/AK/Format.cpp index b1f5e211738..0f8398770fc 100644 --- a/AK/Format.cpp +++ b/AK/Format.cpp @@ -4,9 +4,13 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include +#include #include +#include #include #include +#include #include #include #include @@ -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 Formatter::format_windows_error(FormatBuilder& builder, Error const& error) +{ + thread_local HashMap windows_errors; + + int code = error.code(); + Optional string = windows_errors.get(static_cast(code)); + if (string.has_value()) { + return Formatter::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(code), + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + message, + 0, + nullptr); + if (size == 0) { + auto format_error = GetLastError(); + return Formatter::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::format(builder, string_in_map.view()); +} +#else +ErrorOr Formatter::format_windows_error(FormatBuilder&, Error const&) +{ + VERIFY_NOT_REACHED(); +} +#endif + +ErrorOr Formatter::format(FormatBuilder& builder, Error const& error) +{ + switch (error.kind()) { + case Error::Kind::Syscall: + return Formatter::format(builder, "{}: {} (errno={})"sv, error.string_literal(), strerror(error.code()), error.code()); + case Error::Kind::Errno: + return Formatter::format(builder, "{} (errno={})"sv, strerror(error.code()), error.code()); + case Error::Kind::Windows: + return Formatter::format_windows_error(builder, error); + case Error::Kind::StringLiteral: + return Formatter::format(builder, "{}"sv, error.string_literal()); + } + VERIFY_NOT_REACHED(); +} #ifdef AK_OS_ANDROID static char const* s_log_tag_name = "Serenity"; diff --git a/AK/Format.h b/AK/Format.h index 955471588b3..b9689c5e00b 100644 --- a/AK/Format.h +++ b/AK/Format.h @@ -737,15 +737,10 @@ struct Formatter : Formatter { template<> struct Formatter : Formatter { - ErrorOr format(FormatBuilder& builder, Error const& error) - { - if (error.is_syscall()) - return Formatter::format(builder, "{}: {} (errno={})"sv, error.string_literal(), strerror(error.code()), error.code()); - if (error.is_errno()) - return Formatter::format(builder, "{} (errno={})"sv, strerror(error.code()), error.code()); + ErrorOr format(FormatBuilder& builder, Error const& error); - return Formatter::format(builder, "{}"sv, error.string_literal()); - } +private: + ErrorOr format_windows_error(FormatBuilder& builder, Error const& error); }; template