From d3ee045c2d492388b10518ddca5f8246dc8bab8c Mon Sep 17 00:00:00 2001 From: Patric Stout Date: Wed, 10 Jan 2024 22:38:58 +0100 Subject: [PATCH] Codechange: refactor the Windows-only DllLoader in a cross-platform LibraryLoader (#11751) --- CMakeLists.txt | 1 + src/CMakeLists.txt | 1 + src/library_loader.h | 112 ++++++++++++++++++++++++++ src/os/unix/CMakeLists.txt | 1 + src/os/unix/library_loader_unix.cpp | 64 +++++++++++++++ src/os/windows/CMakeLists.txt | 1 + src/os/windows/crashlog_win.cpp | 23 +++--- src/os/windows/font_win32.cpp | 7 +- src/os/windows/font_win32.h | 2 + src/os/windows/library_loader_win.cpp | 58 +++++++++++++ src/os/windows/win32.cpp | 9 ++- src/os/windows/win32.h | 43 ---------- src/video/win32_v.cpp | 11 +-- src/video/win32_v.h | 1 + 14 files changed, 268 insertions(+), 66 deletions(-) create mode 100644 src/library_loader.h create mode 100644 src/os/unix/library_loader_unix.cpp create mode 100644 src/os/windows/library_loader_win.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 59ae78d043..d1da5bad10 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -311,6 +311,7 @@ link_package(LZO) if(NOT WIN32 AND NOT EMSCRIPTEN) link_package(CURL ENCOURAGED) + target_link_libraries(openttd_lib ${CMAKE_DL_LIBS}) endif() if(NOT EMSCRIPTEN) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 05689fe5c5..2a6275cd35 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -232,6 +232,7 @@ add_files( league_gui.h league_gui.cpp league_type.h + library_loader.h livery.h main_gui.cpp map.cpp diff --git a/src/library_loader.h b/src/library_loader.h new file mode 100644 index 0000000000..a6cc8285cf --- /dev/null +++ b/src/library_loader.h @@ -0,0 +1,112 @@ +/* + * This file is part of OpenTTD. + * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. + * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . + */ + +/** @file library_loader.h Functions/types related to loading libraries dynamically. */ + +#ifndef LIBRARY_LOADER_H +#define LIBRARY_LOADER_H + +class LibraryLoader { +public: + /** + * A function loaded from a library. + * + * Will automatically cast to the correct function pointer type on retrieval. + */ + class Function { + public: + explicit Function(void *p) : p(p) {} + + template >> + operator T *() const + { + return reinterpret_cast(this->p); + } + + private: + void *p; + }; + + /** + * Load a library with the given filename. + */ + explicit LibraryLoader(const std::string &filename) + { + this->handle = this->OpenLibrary(filename); + } + + /** + * Close the library. + */ + ~LibraryLoader() + { + if (this->handle != nullptr) { + this->CloseLibrary(); + } + } + + /** + * Check whether an error occurred while loading the library or a function. + * + * @return Whether an error occurred. + */ + bool HasError() + { + return this->error.has_value(); + } + + /** + * Get the last error that occurred while loading the library or a function. + * + * @return The error message. + */ + std::string GetLastError() + { + return this->error.value_or("No error"); + } + + /** + * Get a function from a loaded library. + * + * @param symbol_name The name of the function to get. + * @return The function. Check HasError() before using. + */ + Function GetFunction(const std::string &symbol_name) + { + if (this->error.has_value()) return Function(nullptr); + return Function(this->GetSymbol(symbol_name)); + } + +private: + /** + * Open the library with the given filename. + * + * Should set error if any error occurred. + * + * @param filename The filename of the library to open. + */ + void *OpenLibrary(const std::string &filename); + + /** + * Close the library. + */ + void CloseLibrary(); + + /** + * Get a symbol from the library. + * + * Should set error if any error occurred. + * + * @param symbol_name The name of the symbol to get. + */ + void *GetSymbol(const std::string &symbol_name); + + std::optional error = {}; ///< The last error that occurred, if set. + void *handle = nullptr; ///< Handle to the library. +}; + +#endif /* LIBRARY_LOADER_H */ diff --git a/src/os/unix/CMakeLists.txt b/src/os/unix/CMakeLists.txt index 1e8bb5d63d..f6c40d015f 100644 --- a/src/os/unix/CMakeLists.txt +++ b/src/os/unix/CMakeLists.txt @@ -5,6 +5,7 @@ add_files( ) add_files( + library_loader_unix.cpp unix.cpp CONDITION UNIX ) diff --git a/src/os/unix/library_loader_unix.cpp b/src/os/unix/library_loader_unix.cpp new file mode 100644 index 0000000000..471f1faa2a --- /dev/null +++ b/src/os/unix/library_loader_unix.cpp @@ -0,0 +1,64 @@ +/* + * This file is part of OpenTTD. + * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. + * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . + */ + +/** @file library_loader_unix.cpp Implementation of the LibraryLoader for Linux / MacOS */ + +#include "../../stdafx.h" + +#include + +#include "../../library_loader.h" + +#include "../../safeguards.h" + +/* Emscripten cannot dynamically load other files. */ +#if defined(__EMSCRIPTEN__) + +void *LibraryLoader::OpenLibrary(const std::string &) +{ + this->error = "Dynamic loading is not supported on this platform."; + return nullptr; +} + +void LibraryLoader::CloseLibrary() +{ +} + +void *LibraryLoader::GetSymbol(const std::string &) +{ + this->error = "Dynamic loading is not supported on this platform."; + return nullptr; +} + +#else + +void *LibraryLoader::OpenLibrary(const std::string &filename) +{ + void *h = dlopen(filename.c_str(), RTLD_NOW | RTLD_LOCAL); + if (h == nullptr) { + this->error = dlerror(); + } + + return h; +} + +void LibraryLoader::CloseLibrary() +{ + dlclose(this->handle); +} + +void *LibraryLoader::GetSymbol(const std::string &symbol_name) +{ + void *p = dlsym(this->handle, symbol_name.c_str()); + if (p == nullptr) { + this->error = dlerror(); + } + + return p; +} + +#endif /* __EMSCRIPTEN__ */ diff --git a/src/os/windows/CMakeLists.txt b/src/os/windows/CMakeLists.txt index 9215514fa2..dd446f7ac6 100644 --- a/src/os/windows/CMakeLists.txt +++ b/src/os/windows/CMakeLists.txt @@ -2,6 +2,7 @@ add_files( crashlog_win.cpp font_win32.cpp font_win32.h + library_loader_win.cpp string_uniscribe.cpp string_uniscribe.h survey_win.cpp diff --git a/src/os/windows/crashlog_win.cpp b/src/os/windows/crashlog_win.cpp index 7c05e81429..e74fc5ed0b 100644 --- a/src/os/windows/crashlog_win.cpp +++ b/src/os/windows/crashlog_win.cpp @@ -17,6 +17,7 @@ #include "../../gamelog.h" #include "../../saveload/saveload.h" #include "../../video/video_driver.hpp" +#include "../../library_loader.h" #include #include @@ -177,7 +178,7 @@ static const uint MAX_FRAMES = 64; /* virtual */ void CrashLogWindows::SurveyStacktrace(nlohmann::json &survey) const { - DllLoader dbghelp(L"dbghelp.dll"); + LibraryLoader dbghelp("dbghelp.dll"); struct ProcPtrs { BOOL (WINAPI * pSymInitialize)(HANDLE, PCSTR, BOOL); BOOL (WINAPI * pSymSetOptions)(DWORD); @@ -189,21 +190,21 @@ static const uint MAX_FRAMES = 64; BOOL (WINAPI * pSymGetSymFromAddr64)(HANDLE, DWORD64, PDWORD64, PIMAGEHLP_SYMBOL64); BOOL (WINAPI * pSymGetLineFromAddr64)(HANDLE, DWORD64, PDWORD, PIMAGEHLP_LINE64); } proc = { - dbghelp.GetProcAddress("SymInitialize"), - dbghelp.GetProcAddress("SymSetOptions"), - dbghelp.GetProcAddress("SymCleanup"), - dbghelp.GetProcAddress("StackWalk64"), - dbghelp.GetProcAddress("SymFunctionTableAccess64"), - dbghelp.GetProcAddress("SymGetModuleBase64"), - dbghelp.GetProcAddress("SymGetModuleInfo64"), - dbghelp.GetProcAddress("SymGetSymFromAddr64"), - dbghelp.GetProcAddress("SymGetLineFromAddr64"), + dbghelp.GetFunction("SymInitialize"), + dbghelp.GetFunction("SymSetOptions"), + dbghelp.GetFunction("SymCleanup"), + dbghelp.GetFunction("StackWalk64"), + dbghelp.GetFunction("SymFunctionTableAccess64"), + dbghelp.GetFunction("SymGetModuleBase64"), + dbghelp.GetFunction("SymGetModuleInfo64"), + dbghelp.GetFunction("SymGetSymFromAddr64"), + dbghelp.GetFunction("SymGetLineFromAddr64"), }; survey = nlohmann::json::array(); /* Try to load the functions from the DLL, if that fails because of a too old dbghelp.dll, just skip it. */ - if (dbghelp.Success()) { + if (!dbghelp.HasError()) { /* Initialize symbol handler. */ HANDLE hCur = GetCurrentProcess(); proc.pSymInitialize(hCur, nullptr, TRUE); diff --git a/src/os/windows/font_win32.cpp b/src/os/windows/font_win32.cpp index 3d863b4dc0..a05b2211a6 100644 --- a/src/os/windows/font_win32.cpp +++ b/src/os/windows/font_win32.cpp @@ -15,9 +15,10 @@ #include "../../core/mem_func.hpp" #include "../../error_func.h" #include "../../fileio_func.h" -#include "../../fontdetection.h" #include "../../fontcache.h" #include "../../fontcache/truetypefontcache.h" +#include "../../fontdetection.h" +#include "../../library_loader.h" #include "../../string_func.h" #include "../../strings_func.h" #include "../../zoom_func.h" @@ -361,9 +362,9 @@ void LoadWin32Font(FontSize fs) if (AddFontResourceEx(fontPath, FR_PRIVATE, 0) != 0) { /* Try a nice little undocumented function first for getting the internal font name. * Some documentation is found at: http://www.undocprint.org/winspool/getfontresourceinfo */ - static DllLoader _gdi32(L"gdi32.dll"); + static LibraryLoader _gdi32("gdi32.dll"); typedef BOOL(WINAPI *PFNGETFONTRESOURCEINFO)(LPCTSTR, LPDWORD, LPVOID, DWORD); - static PFNGETFONTRESOURCEINFO GetFontResourceInfo = _gdi32.GetProcAddress("GetFontResourceInfoW"); + static PFNGETFONTRESOURCEINFO GetFontResourceInfo = _gdi32.GetFunction("GetFontResourceInfoW"); if (GetFontResourceInfo != nullptr) { /* Try to query an array of LOGFONTs that describe the file. */ diff --git a/src/os/windows/font_win32.h b/src/os/windows/font_win32.h index 7ef5568601..02a84b63fc 100644 --- a/src/os/windows/font_win32.h +++ b/src/os/windows/font_win32.h @@ -13,6 +13,8 @@ #include "../../fontcache/truetypefontcache.h" #include "win32.h" +#include + /** Font cache for fonts that are based on a Win32 font. */ class Win32FontCache : public TrueTypeFontCache { private: diff --git a/src/os/windows/library_loader_win.cpp b/src/os/windows/library_loader_win.cpp new file mode 100644 index 0000000000..a682f2e7f1 --- /dev/null +++ b/src/os/windows/library_loader_win.cpp @@ -0,0 +1,58 @@ +/* + * This file is part of OpenTTD. + * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. + * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . + */ + +/** @file library_loader_win.cpp Implementation of the LibraryLoader for Windows */ + +#include "../../stdafx.h" + +#include + +#include "../../library_loader.h" + +#include "../../safeguards.h" + +static std::string GetLoadError() +{ + auto error_code = GetLastError(); + + char buffer[512]; + if (FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, error_code, + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), buffer, sizeof(buffer), nullptr) == 0) { + return fmt::format("Unknown error {}", error_code); + } + + return buffer; +} + +void *LibraryLoader::OpenLibrary(const std::string &filename) +{ + void *h = ::LoadLibraryW(OTTD2FS(filename).c_str()); + if (h == nullptr) { + this->error = GetLoadError(); + } + + return h; +} + +void LibraryLoader::CloseLibrary() +{ + HMODULE handle = static_cast(this->handle); + + ::FreeLibrary(handle); +} + +void *LibraryLoader::GetSymbol(const std::string &symbol_name) +{ + HMODULE handle = static_cast(this->handle); + + void *p = reinterpret_cast(::GetProcAddress(handle, symbol_name.c_str())); + if (p == nullptr) { + this->error = GetLoadError(); + } + + return p; +} diff --git a/src/os/windows/win32.cpp b/src/os/windows/win32.cpp index 1d4e747378..b198ed92c4 100644 --- a/src/os/windows/win32.cpp +++ b/src/os/windows/win32.cpp @@ -27,6 +27,7 @@ #include #include "../../language.h" #include "../../thread.h" +#include "../../library_loader.h" #include "../../safeguards.h" @@ -573,8 +574,8 @@ int OTTDStringCompare(std::string_view s1, std::string_view s2) #endif if (first_time) { - static DllLoader _kernel32(L"Kernel32.dll"); - _CompareStringEx = _kernel32.GetProcAddress("CompareStringEx"); + static LibraryLoader _kernel32("Kernel32.dll"); + _CompareStringEx = _kernel32.GetFunction("CompareStringEx"); first_time = false; } @@ -617,8 +618,8 @@ int Win32StringContains(const std::string_view str, const std::string_view value static bool first_time = true; if (first_time) { - static DllLoader _kernel32(L"Kernel32.dll"); - _FindNLSStringEx = _kernel32.GetProcAddress("FindNLSStringEx"); + static LibraryLoader _kernel32("Kernel32.dll"); + _FindNLSStringEx = _kernel32.GetFunction("FindNLSStringEx"); first_time = false; } diff --git a/src/os/windows/win32.h b/src/os/windows/win32.h index 561d471bac..4e35241721 100644 --- a/src/os/windows/win32.h +++ b/src/os/windows/win32.h @@ -10,51 +10,8 @@ #ifndef WIN32_H #define WIN32_H -#include bool MyShowCursor(bool show, bool toggle = false); -class DllLoader { -public: - explicit DllLoader(LPCTSTR filename) - { - this->hmodule = ::LoadLibrary(filename); - if (this->hmodule == nullptr) this->success = false; - } - - - ~DllLoader() - { - ::FreeLibrary(this->hmodule); - } - - bool Success() { return this->success; } - - class ProcAddress { - public: - explicit ProcAddress(void *p) : p(p) {} - - template >> - operator T *() const - { - return reinterpret_cast(this->p); - } - - private: - void *p; - }; - - ProcAddress GetProcAddress(const char *proc_name) - { - void *p = reinterpret_cast(::GetProcAddress(this->hmodule, proc_name)); - if (p == nullptr) this->success = false; - return ProcAddress(p); - } - -private: - HMODULE hmodule = nullptr; - bool success = true; -}; - char *convert_from_fs(const wchar_t *name, char *utf8_buf, size_t buflen); wchar_t *convert_to_fs(const std::string_view name, wchar_t *utf16_buf, size_t buflen); diff --git a/src/video/win32_v.cpp b/src/video/win32_v.cpp index 833f4bf72d..a2710186a2 100644 --- a/src/video/win32_v.cpp +++ b/src/video/win32_v.cpp @@ -22,6 +22,7 @@ #include "../window_gui.h" #include "../window_func.h" #include "../framerate_type.h" +#include "../library_loader.h" #include "win32_v.h" #include #include @@ -976,11 +977,11 @@ float VideoDriver_Win32Base::GetDPIScale() static bool init_done = false; if (!init_done) { init_done = true; - static DllLoader _user32(L"user32.dll"); - static DllLoader _shcore(L"shcore.dll"); - _GetDpiForWindow = _user32.GetProcAddress("GetDpiForWindow"); - _GetDpiForSystem = _user32.GetProcAddress("GetDpiForSystem"); - _GetDpiForMonitor = _shcore.GetProcAddress("GetDpiForMonitor"); + static LibraryLoader _user32("user32.dll"); + static LibraryLoader _shcore("shcore.dll"); + _GetDpiForWindow = _user32.GetFunction("GetDpiForWindow"); + _GetDpiForSystem = _user32.GetFunction("GetDpiForSystem"); + _GetDpiForMonitor = _shcore.GetFunction("GetDpiForMonitor"); } UINT cur_dpi = 0; diff --git a/src/video/win32_v.h b/src/video/win32_v.h index 8e9493df8f..272ee607a9 100644 --- a/src/video/win32_v.h +++ b/src/video/win32_v.h @@ -13,6 +13,7 @@ #include "video_driver.hpp" #include #include +#include /** Base class for Windows video drivers. */ class VideoDriver_Win32Base : public VideoDriver {