From 9fcc3afd3c6f80df00a58b647cc08b1a8d4a2aa1 Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Sat, 13 Apr 2024 20:12:32 -0700 Subject: [PATCH] Clean up some asserts and add an alassert Unlike a normal assert, alassert doesn't compile away in Release/NDEBUG builds, making it useful for checks that must be true for continuing operation but may not be due to unforeseeable/extenuating circumstances. --- CMakeLists.txt | 2 ++ al/auxeffectslot.cpp | 2 -- al/source.cpp | 16 ++++++---------- al/source.h | 2 +- common/alassert.cpp | 37 +++++++++++++++++++++++++++++++++++++ common/alassert.h | 22 ++++++++++++++++++++++ common/alspan.h | 12 ++++++------ 7 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 common/alassert.cpp create mode 100644 common/alassert.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 10044e84..9a055bc8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -608,6 +608,8 @@ endif() # Common sources used by both the OpenAL implementation library, the OpenAL # router, and certain tools and examples. set(COMMON_OBJS + common/alassert.cpp + common/alassert.h common/albit.h common/alcomplex.cpp common/alcomplex.h diff --git a/al/auxeffectslot.cpp b/al/auxeffectslot.cpp index ec7fd776..a803500f 100644 --- a/al/auxeffectslot.cpp +++ b/al/auxeffectslot.cpp @@ -23,7 +23,6 @@ #include "auxeffectslot.h" #include -#include #include #include #include @@ -1528,7 +1527,6 @@ void ALeffectslot::eax_set_efx_slot_gain(ALfloat gain) void ALeffectslot::EaxDeleter::operator()(ALeffectslot* effect_slot) { - assert(effect_slot); eax_delete_al_effect_slot(*effect_slot->eax_al_context_, *effect_slot); } diff --git a/al/source.cpp b/al/source.cpp index d8c9b47f..fd711b61 100644 --- a/al/source.cpp +++ b/al/source.cpp @@ -4570,13 +4570,11 @@ void ALsource::eax_set(const EaxCall& call) mEaxVersion = eax_version; } -void ALsource::eax_get_active_fx_slot_id(const EaxCall& call, const GUID* ids, size_t max_count) +void ALsource::eax_get_active_fx_slot_id(const EaxCall& call, const al::span src_ids) { - assert(ids != nullptr); - assert(max_count == EAX40_MAX_ACTIVE_FXSLOTS || max_count == EAX50_MAX_ACTIVE_FXSLOTS); - const auto dst_ids = call.get_values(max_count); - const auto count = dst_ids.size(); - std::uninitialized_copy_n(ids, count, dst_ids.begin()); + assert(src_ids.size()==EAX40_MAX_ACTIVE_FXSLOTS || src_ids.size()==EAX50_MAX_ACTIVE_FXSLOTS); + const auto dst_ids = call.get_values(src_ids.size()); + std::uninitialized_copy_n(src_ids.begin(), dst_ids.size(), dst_ids.begin()); } void ALsource::eax1_get(const EaxCall& call, const Eax1Props& props) @@ -4824,8 +4822,7 @@ void ALsource::eax4_get(const EaxCall& call, const Eax4Props& props) break; case EAXSOURCE_ACTIVEFXSLOTID: - eax_get_active_fx_slot_id(call, props.active_fx_slots.guidActiveFXSlots.data(), - EAX40_MAX_ACTIVE_FXSLOTS); + eax_get_active_fx_slot_id(call, props.active_fx_slots.guidActiveFXSlots); break; default: @@ -4897,8 +4894,7 @@ void ALsource::eax5_get(const EaxCall& call, const Eax5Props& props) break; case EAXSOURCE_ACTIVEFXSLOTID: - eax_get_active_fx_slot_id(call, props.active_fx_slots.guidActiveFXSlots.data(), - EAX50_MAX_ACTIVE_FXSLOTS); + eax_get_active_fx_slot_id(call, props.active_fx_slots.guidActiveFXSlots); break; case EAXSOURCE_MACROFXFACTOR: diff --git a/al/source.h b/al/source.h index 7c81e864..8ec68538 100644 --- a/al/source.h +++ b/al/source.h @@ -889,7 +889,7 @@ private: } } - static void eax_get_active_fx_slot_id(const EaxCall& call, const GUID* ids, size_t max_count); + static void eax_get_active_fx_slot_id(const EaxCall& call, const al::span src_ids); static void eax1_get(const EaxCall& call, const Eax1Props& props); static void eax2_get(const EaxCall& call, const Eax2Props& props); static void eax3_get_obstruction(const EaxCall& call, const Eax3Props& props); diff --git a/common/alassert.cpp b/common/alassert.cpp new file mode 100644 index 00000000..2eab2520 --- /dev/null +++ b/common/alassert.cpp @@ -0,0 +1,37 @@ + +#include "alassert.h" + +#include +#include + +namespace al { + +[[noreturn]] +void do_assert(const char *message, int linenum, const char *filename, const char *funcname) noexcept +{ + std::string errstr{filename}; + errstr += ':'; + errstr += std::to_string(linenum); + errstr += ": "; + errstr += funcname; + errstr += ": "; + errstr += message; + /* Calling std::terminate in a catch block hopefully causes the system to + * provide info about the caught exception in the error dialog. At least on + * Linux, this results in the process printing + * + * terminate called after throwing an instance of 'std::runtime_error' + * what(): + * + * before terminating from a SIGABRT. Hopefully Windows and Mac will do the + * appropriate things with the message for an abnormal termination. + */ + try { + throw std::runtime_error{errstr}; + } + catch(...) { + std::terminate(); + } +} + +} /* namespace al */ diff --git a/common/alassert.h b/common/alassert.h new file mode 100644 index 00000000..a9290160 --- /dev/null +++ b/common/alassert.h @@ -0,0 +1,22 @@ +#ifndef AL_ASSERT_H +#define AL_ASSERT_H + +#include "opthelpers.h" + +namespace al { + +[[noreturn]] +void do_assert(const char *message, int linenum, const char *filename, const char *funcname) noexcept; + +} /* namespace al */ + +/* A custom assert macro that is not compiled out for Release/NDEBUG builds, + * making it an appropriate replacement for assert() checks that must not be + * ignored. + */ +#define alassert(cond) do { \ + if(!(cond)) UNLIKELY \ + al::do_assert("Assertion '" #cond "' failed", __LINE__, __FILE__, __func__); \ +} while(0) + +#endif /* AL_ASSERT_H */ diff --git a/common/alspan.h b/common/alspan.h index 0156e6e7..c60aeeb8 100644 --- a/common/alspan.h +++ b/common/alspan.h @@ -9,6 +9,7 @@ #include #include +#include "alassert.h" #include "almalloc.h" #include "altraits.h" @@ -164,12 +165,11 @@ public: template constexpr span() noexcept { } template - constexpr explicit span(U iter, size_type size_ [[maybe_unused]]) - : mData{::al::to_address(iter)} - { assert(size_ == extent); } + constexpr explicit span(U iter, size_type size_) : mData{::al::to_address(iter)} + { alassert(size_ == extent); } template::value)> - constexpr explicit span(U first, V last [[maybe_unused]]) : mData{::al::to_address(first)} - { assert(static_cast(last-first) == extent); } + constexpr explicit span(U first, V last) : mData{::al::to_address(first)} + { alassert(static_cast(last-first) == extent); } template constexpr span(type_identity_t (&arr)[N]) noexcept /* NOLINT(*-avoid-c-arrays) */ @@ -188,7 +188,7 @@ public: template::value && detail_::is_array_compatible && N == dynamic_extent)> constexpr explicit span(const span &span_) noexcept : mData{std::data(span_)} - { assert(std::size(span_) == extent); } + { alassert(std::size(span_) == extent); } template::value && detail_::is_array_compatible && N == extent)> constexpr span(const span &span_) noexcept : mData{std::data(span_)} { } -- 2.11.4.GIT