From e9dc22a399f4dc5ce1c7759b9f7945825c293a5d Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 30 Dec 2022 20:17:26 -0800 Subject: [PATCH 1/3] Rename strcasestr to nongnu_strcasestr, macro strcasestr ifndef _GNU_SOURCE Fix for GCC C++ compiler, which always defines _GNU_SOURCE. --- src/doomtype.h | 5 ++++- src/strcasestr.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/doomtype.h b/src/doomtype.h index b1269eaff..d08850f8e 100644 --- a/src/doomtype.h +++ b/src/doomtype.h @@ -110,7 +110,10 @@ typedef long ssize_t; #define strnicmp(x,y,n) strncasecmp(x,y,n) #endif -char *strcasestr(const char *in, const char *what); +char *nongnu_strcasestr(const char *in, const char *what); +#ifndef _GNU_SOURCE +#define strcasestr nongnu_strcasestr +#endif #define stristr strcasestr #if defined (PC_DOS) || defined (_WIN32) || defined (__HAIKU__) diff --git a/src/strcasestr.c b/src/strcasestr.c index b266278ed..4904fb664 100644 --- a/src/strcasestr.c +++ b/src/strcasestr.c @@ -51,7 +51,7 @@ swapp (char ***ppap, char ***ppbp, char **cpap, char **cpbp) } char * -strcasestr (const char *s, const char *q) +nongnu_strcasestr (const char *s, const char *q) { size_t qn; From 6af003771dc9326a3226829f54b7498ff49f2c2b Mon Sep 17 00:00:00 2001 From: Eidolon Date: Wed, 28 Dec 2022 22:01:47 -0600 Subject: [PATCH 2/3] Add SRB2_ASSERT, srb2::NotNull Add SRB2_ASSERT, superceding I_Assert This assertion macro always expands to a call of srb2::do_assert, which is overloaded with two templates: one which applies if the provided Level is less than or equal to the SRB2_ASSERTION_LEVEL, and one which is a no-op. When optimizations are enabled, this will verifiably remove the evaluation of the expression in all cases, instead of evaluating the expression and doing nothing with it. Add srb2::NotNull wrapper utility This is meant to be used in places where pointers are used as parameters. It can be used with any pointer-like type, not just raw pointers. During construction of NotNull, the pointer will be asserted not-null in debug and paranoia builds, and in release optimizations with no assertions, the code decays gracefully to standard pointer-passing. --- src/cxxutil.hpp | 117 ++++++++++++++++++++++++++++++++++++++- src/doomdef.h | 1 + src/tests/CMakeLists.txt | 2 + src/tests/boolcompat.cpp | 1 + src/tests/notnull.cpp | 31 +++++++++++ src/tests/testbase.hpp | 6 ++ 6 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 src/tests/notnull.cpp create mode 100644 src/tests/testbase.hpp diff --git a/src/cxxutil.hpp b/src/cxxutil.hpp index befe9c20b..74349a0ff 100644 --- a/src/cxxutil.hpp +++ b/src/cxxutil.hpp @@ -1,9 +1,12 @@ #ifndef __SRB2_CXXUTIL_HPP__ #define __SRB2_CXXUTIL_HPP__ +#include #include #include +#include "doomdef.h" + namespace srb2 { template @@ -19,7 +22,8 @@ public: void operator=(Finally&& from) = delete; ~Finally() noexcept { - f_(); + if (call_) + f_(); } private: @@ -32,6 +36,117 @@ Finally> finally(F&& f) noexcept { return Finally {std::forward(f)}; } +struct SourceLocation { + const char* file_name; + unsigned int line_number; +}; + +#define SRB2_SOURCE_LOCATION \ + srb2::SourceLocation { __FILE__, __LINE__ } + +#ifndef SRB2_ASSERT_HANDLER +#define SRB2_ASSERT_HANDLER srb2::IErrorAssertHandler +#endif + +#if !defined(NDEBUG) || defined(PARANOIA) +// An assertion level of 2 will activate all invocations of the SRB2_ASSERT macro +#define SRB2_ASSERTION_LEVEL 2 +#else +// The minimum assertion level is 1 +#define SRB2_ASSERTION_LEVEL 1 +#endif + +/// Assert a precondition expression in debug builds. +#define SRB2_ASSERT(expr) srb2::do_assert<2, SRB2_ASSERT_HANDLER>([&] { return (expr); }, SRB2_SOURCE_LOCATION, #expr) + +class IErrorAssertHandler { +public: + static void handle(const SourceLocation& source_location, const char* expression) { + I_Error("Assertion failed at %s:%u: %s != true", + source_location.file_name, + source_location.line_number, + expression); + } +}; + +class NoOpAssertHandler { +public: + static void handle(const SourceLocation& source_location, const char* expression) {} +}; + +/// @brief Assert a precondition expression, aborting the application if it fails. +/// @tparam Expr +/// @tparam Level the level of this assertion; if it is less than or equal to SRB2_ASSERTION_LEVEL, this overload will +/// activate. +/// @param expr a callable which returns a bool +/// @param source_location a struct containing the source location of the assertion, e.g. SRB2_SOURCE_LOCATION +/// @param expression the expression evaluated in the expression callable +/// @param message an optional message to display for the assertion +template +std::enable_if_t<(Level <= SRB2_ASSERTION_LEVEL), void> +do_assert(const Expr& expr, const SourceLocation& source_location, const char* expression = "") noexcept { + static_assert(Level > 0, "level of an assertion must not be 0"); + if (!expr()) { + Handler::handle(source_location, expression); + std::abort(); + } } +template +std::enable_if_t<(Level > SRB2_ASSERTION_LEVEL), void> +do_assert(const Expr&, const SourceLocation&, const char* = "") noexcept { +} + +template +class NotNull final { + T ptr_; + +public: + static_assert(std::is_convertible_v() != nullptr), bool>, + "T is not comparable with nullptr_t"); + + /// @brief Move-construct from the pointer value U, asserting that it is not null. Allows construction of a + /// NotNull from any compatible pointer U, for example with polymorphic classes. + template >> + constexpr NotNull(U&& rhs) : ptr_(std::forward(rhs)) { + SRB2_ASSERT(ptr_ != nullptr); + } + + /// @brief Wrap the pointer type T, asserting that the pointer is not null. + template >> + constexpr NotNull(T rhs) : ptr_(std::move(rhs)) { + SRB2_ASSERT(ptr_ != nullptr); + } + + /// @brief Copy construction from NotNull of convertible type U. Only if the incoming pointer is NotNull already. + template >> + constexpr NotNull(const NotNull& rhs) : NotNull(rhs.get()) { + // Value is guaranteed to be not null by construction; no assertion necessary + } + + NotNull(const NotNull& rhs) = default; + NotNull& operator=(const NotNull& rhs) = default; + + /// @brief Get the stored pointer. + constexpr T get() const { return ptr_; } + + /// @brief Convert to T (the pointer type). + constexpr operator T() const { return get(); } + + /// @brief Arrow-dereference to *T (the actual value pointed to). + constexpr decltype(auto) operator->() const { return get(); } + + /// @brief Dereference to *T (the actual value pointed to). + constexpr decltype(auto) operator*() const { return *get(); } + + // It is not allowed to construct NotNull with nullptr regardless of T. + NotNull(std::nullptr_t) = delete; + NotNull& operator=(std::nullptr_t) = delete; +}; + +template +NotNull(T) -> NotNull; + +} // namespace srb2 + #endif // __SRB2_CXXUTIL_HPP__ diff --git a/src/doomdef.h b/src/doomdef.h index 31892fb28..09a5efa4a 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -645,6 +645,7 @@ UINT32 quickncasehash (const char *p, size_t n) #endif // An assert-type mechanism. +// NOTE: USE SRB2_ASSERT FOR C++ CODE INSTEAD #ifdef PARANOIA #define I_Assert(e) ((e) ? (void)0 : I_Error("assert failed: %s, file %s, line %d", #e, __FILE__, __LINE__)) #else diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 28c4ce492..59c64ae00 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -1,3 +1,5 @@ target_sources(srb2tests PRIVATE boolcompat.cpp + notnull.cpp + testbase.hpp ) diff --git a/src/tests/boolcompat.cpp b/src/tests/boolcompat.cpp index fee40cd36..72e2ccba7 100644 --- a/src/tests/boolcompat.cpp +++ b/src/tests/boolcompat.cpp @@ -1,3 +1,4 @@ +#include "testbase.hpp" #include #include "../doomtype.h" diff --git a/src/tests/notnull.cpp b/src/tests/notnull.cpp new file mode 100644 index 000000000..282ef9e57 --- /dev/null +++ b/src/tests/notnull.cpp @@ -0,0 +1,31 @@ +#include "testbase.hpp" +#include + +#include "../cxxutil.hpp" + +namespace { +class A { +public: + virtual bool foo() = 0; +}; +class B : public A { +public: + virtual bool foo() override final { return true; }; +}; +} // namespace + +TEST_CASE("NotNull is constructible from int*") { + int a = 0; + REQUIRE(srb2::NotNull(static_cast(&a))); +} + +TEST_CASE("NotNull is constructible from B* where B inherits from A") { + B b; + REQUIRE(srb2::NotNull(static_cast(&b))); +} + +TEST_CASE("NotNull dereferences to B& to call foo") { + B b; + srb2::NotNull a {static_cast(&b)}; + REQUIRE(a->foo()); +} diff --git a/src/tests/testbase.hpp b/src/tests/testbase.hpp new file mode 100644 index 000000000..39ce223a0 --- /dev/null +++ b/src/tests/testbase.hpp @@ -0,0 +1,6 @@ +#ifndef __SRB2_TESTS_TESTBASE_HPP__ +#define __SRB2_TESTS_TESTBASE_HPP__ + +#define SRB2_ASSERT_HANDLER srb2::NoOpAssertHandler + +#endif // __SRB2_TESTS_TESTBASE_HPP__ From 19c57110927d61904babe587dd7a3ebb35abac29 Mon Sep 17 00:00:00 2001 From: Eidolon Date: Sun, 1 Jan 2023 14:48:55 -0600 Subject: [PATCH 3/3] cmake: build discord-rpc internally --- CMakeLists.txt | 1 - cmake/Modules/FindDiscordRPC.cmake | 33 ----------- thirdparty/CMakeLists.txt | 90 +++++++++++++++++++++++++----- 3 files changed, 77 insertions(+), 47 deletions(-) delete mode 100644 cmake/Modules/FindDiscordRPC.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 10cf0d022..3dfd8e6f9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -134,7 +134,6 @@ if("${SRB2_CONFIG_SYSTEM_LIBRARIES}") find_package(CURL REQUIRED) find_package(OPENMPT REQUIRED) find_package(GME REQUIRED) - find_package(DiscordRPC REQUIRED) endif() if(${PROJECT_SOURCE_DIR} MATCHES ${PROJECT_BINARY_DIR}) diff --git a/cmake/Modules/FindDiscordRPC.cmake b/cmake/Modules/FindDiscordRPC.cmake deleted file mode 100644 index 2d3c987c1..000000000 --- a/cmake/Modules/FindDiscordRPC.cmake +++ /dev/null @@ -1,33 +0,0 @@ -include(LibFindMacros) - -libfind_pkg_check_modules(DISCORDRPC_PKGCONF DISCORDRPC) - -find_path(DISCORDRPC_INCLUDE_DIR - NAMES discord_rpc.h - PATHS - ${DISCORDRPC_PKGCONF_INCLUDE_DIRS} - "/usr/include" - "/usr/local/include" -) - -find_library(DISCORDRPC_LIBRARY - NAMES discord-rpc - PATHS - ${DISCORDRPC_PKGCONF_LIBRARY_DIRS} - "/usr/lib" - "/usr/local/lib" -) - -set(DISCORDRPC_PROCESS_INCLUDES DISCORDRPC_INCLUDE_DIR) -set(DISCORDRPC_PROCESS_LIBS DISCORDRPC_LIBRARY) -libfind_process(DISCORDRPC) - -if(DISCORDRPC_FOUND AND NOT TARGET DiscordRPC::DiscordRPC) - add_library(DiscordRPC::DiscordRPC UNKNOWN IMPORTED) - set_target_properties( - DiscordRPC::DiscordRPC - PROPERTIES - IMPORTED_LOCATION "${DISCORDRPC_LIBRARY}" - INTERFACE_INCLUDE_DIRECTORIES "${DISCORDRPC_INCLUDE_DIR}" - ) -endif() diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index ce5f58531..142bd2491 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -88,7 +88,7 @@ if(NOT "${SRB2_CONFIG_SYSTEM_LIBRARIES}") zutil.c ) list(TRANSFORM ZLIB_SRCS PREPEND "${ZLIB_SOURCE_DIR}/") - + configure_file("${ZLIB_SOURCE_DIR}/zlib.pc.cmakein" "${ZLIB_BINARY_DIR}/zlib.pc" @ONLY) configure_file("${ZLIB_SOURCE_DIR}/zconf.h.cmakein" "${ZLIB_BINARY_DIR}/include/zconf.h" @ONLY) configure_file("${ZLIB_SOURCE_DIR}/zlib.h" "${ZLIB_BINARY_DIR}/include/zlib.h" @ONLY) @@ -123,7 +123,7 @@ if(NOT "${SRB2_CONFIG_SYSTEM_LIBRARIES}") png.h pngconf.h - + pngpriv.h pngdebug.h pnginfo.h @@ -472,7 +472,7 @@ if(NOT "${SRB2_CONFIG_SYSTEM_LIBRARIES}") soundlib/OggStream.h soundlib/Loaders.h soundlib/BitReader.h - soundlib/opal.h + soundlib/opal.h sounddsp/AGC.cpp sounddsp/EQ.cpp @@ -501,7 +501,7 @@ if(NOT "${SRB2_CONFIG_SYSTEM_LIBRARIES}") endif() target_compile_features(openmpt PRIVATE cxx_std_11) target_compile_definitions(openmpt PRIVATE -DLIBOPENMPT_BUILD) - + target_include_directories(openmpt PRIVATE "${openmpt_SOURCE_DIR}/common") target_include_directories(openmpt PRIVATE "${openmpt_SOURCE_DIR}/src") target_include_directories(openmpt PRIVATE "${openmpt_SOURCE_DIR}/include") @@ -528,17 +528,81 @@ if(NOT "${SRB2_CONFIG_SYSTEM_LIBRARIES}") target_link_libraries(gme PRIVATE ZLIB::ZLIB) endif() -if(NOT "${SRB2_CONFIG_SYSTEM_LIBRARIES}") - CPMAddPackage( - NAME DiscordRPC - VERSION 3.4.0 - URL "https://github.com/discord/discord-rpc/archive/refs/tags/v3.4.0.zip" - EXCLUDE_FROM_ALL ON - OPTIONS - "BUILD_EXAMPLES OFF" +CPMAddPackage( + NAME RapidJSON + VERSION 1.1.0 + URL "https://github.com/Tencent/rapidjson/archive/v1.1.0.tar.gz" + EXCLUDE_FROM_ALL ON + DOWNLOAD_ONLY ON +) +if(RapidJSON_ADDED) + add_library(RapidJSON INTERFACE) + add_library(RapidJSON::RapidJSON ALIAS RapidJSON) + target_include_directories(RapidJSON INTERFACE "${RapidJSON_SOURCE_DIR}/include") +endif() + +CPMAddPackage( + NAME DiscordRPC + VERSION 3.4.0 + URL "https://github.com/discord/discord-rpc/archive/refs/tags/v3.4.0.zip" + EXCLUDE_FROM_ALL ON + DOWNLOAD_ONLY ON +) + +if(DiscordRPC_ADDED) + set(DiscordRPC_SOURCES + include/discord_rpc.h + include/discord_register.h + + src/discord_rpc.cpp + src/rpc_connection.h + src/rpc_connection.cpp + src/serialization.h + src/serialization.cpp + src/connection.h + src/backoff.h + src/msg_queue.h ) - target_include_directories(discord-rpc INTERFACE "${DiscordRPC_SOURCE_DIR}/include") + list(TRANSFORM DiscordRPC_SOURCES PREPEND "${DiscordRPC_SOURCE_DIR}/") + + # Discord RPC is always statically linked because it's tiny. + add_library(discord-rpc STATIC ${DiscordRPC_SOURCES}) add_library(DiscordRPC::DiscordRPC ALIAS discord-rpc) + + target_include_directories(discord-rpc PUBLIC "${DiscordRPC_SOURCE_DIR}/include") + target_compile_features(discord-rpc PUBLIC cxx_std_11) + target_link_libraries(discord-rpc PRIVATE RapidJSON::RapidJSON) + + # Platform-specific connection and register impls + if(WIN32) + target_compile_definitions(discord-rpc PUBLIC -DDISCORD_WINDOWS) + target_sources(discord-rpc PRIVATE + "${DiscordRPC_SOURCE_DIR}/src/connection_win.cpp" + "${DiscordRPC_SOURCE_DIR}/src/discord_register_win.cpp" + ) + target_link_libraries(discord-rpc PRIVATE psapi advapi32) + endif() + + if(UNIX) + target_sources(discord-rpc PRIVATE + "${DiscordRPC_SOURCE_DIR}/src/connection_unix.cpp" + ) + + if(APPLE) + target_compile_definitions(discord-rpc PUBLIC -DDISCORD_OSX) + target_sources(discord-rpc PRIVATE + "${DiscordRPC_SOURCE_DIR}/src/discord_register_osx.m" + ) + target_link_libraries(discord-rpc PUBLIC "-framework AppKit") + endif() + + if(UNIX AND NOT APPLE) + target_compile_definitions(discord-rpc PUBLIC -DDISCORD_LINUX) + target_sources(discord-rpc PRIVATE + "${DiscordRPC_SOURCE_DIR}/src/discord_register_linux.cpp" + ) + endif() + endif() endif() add_subdirectory(tcbrindle_span)