From 71c6e2f7144fb0b30ba01fcccdba8b0172664dad Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Sat, 6 Jan 2024 01:20:48 +0100 Subject: [PATCH] Fix and modernize listener<> Commit d1f50af71316b3ab817f0438aecd0a639a93af63 broke the listener<> template in a subtle way: the destructor wouldn't remove listeners from the set because dynamic_cast to a derived type returns nullptr in a destructor. This caused a segfault upon saving map images. In order to solve both the warning that lead to the commit and the new bug, change the set of instances to hold listener* instead of A*, and use dynamic_cast in invoke since the A objects are still valid there. While I was there, I changed invoke() to be a variadic template. It wasn't one because the code originally had to support C++03. --- client/chatline.h | 2 -- client/listener.h | 77 +++++++++-------------------------------------- 2 files changed, 15 insertions(+), 64 deletions(-) diff --git a/client/chatline.h b/client/chatline.h index b4cf8f2223..4385276ae0 100644 --- a/client/chatline.h +++ b/client/chatline.h @@ -27,12 +27,10 @@ class QPaintEvent; class QPainter; class QPushButton; class QUrl; -class chat_listener; void set_chat_colors(const QHash &colors); QString apply_tags(QString str, const struct text_tag_list *tags, QColor bg_color); -template <> std::set listener::instances; /*************************************************************************** Listener for chat. See listener<> for information about how to use it ***************************************************************************/ diff --git a/client/listener.h b/client/listener.h index 6bf5dccd80..a801003185 100644 --- a/client/listener.h +++ b/client/listener.h @@ -10,6 +10,7 @@ #pragma once #include +#include /*************************************************************************** Helper template to connect C and C++ code. @@ -103,7 +104,7 @@ template class listener { private: // All instances of type_t that have called listen(). - static std::set instances; + static std::set *> instances; protected: explicit listener(); @@ -112,13 +113,8 @@ template class listener { void listen(); public: - template static void invoke(_member_fct_ function); - - template - static void invoke(_member_fct_ function, _arg1_t_ arg); - - template - static void invoke(_member_fct_ function, _arg1_t_ arg1, _arg2_t_ arg2); + template + static void invoke(_member_fct_ function, _args_ &&...args); }; /*************************************************************************** @@ -126,7 +122,8 @@ template class listener { ***************************************************************************/ #define FC_CPP_DECLARE_LISTENER(_type_) \ template <> \ - std::set<_type_ *> listener<_type_>::instances = std::set<_type_ *>(); + std::set *> listener<_type_>::instances = \ + std::set *>(); /*************************************************************************** Constructor @@ -140,7 +137,7 @@ template void listener<_type_>::listen() { // If you get an error here, your listener likely doesn't inherit from the // listener<> correctly. See the class documentation. - instances.insert(dynamic_cast(this)); + instances.insert(this); } /*************************************************************************** @@ -148,66 +145,22 @@ template void listener<_type_>::listen() ***************************************************************************/ template listener<_type_>::~listener() { - instances.erase(dynamic_cast(this)); -} - -/*************************************************************************** - Invokes a member function on all instances of an listener type. Template - parameters are meant to be automatically deduced. - - Zero-parameter overload. - - @param function The member function to call -***************************************************************************/ -template -template -void listener<_type_>::invoke(_member_fct_ function) -{ - typename std::set::iterator it = instances.begin(); - typename std::set::iterator end = instances.end(); - for (; it != end; ++it) { - ((*it)->*function)(); - } + instances.erase(this); } /*************************************************************************** - Invokes a member function on all instances of an listener type. Template + Invokes a member function on all instances of a listener type. Template parameters are meant to be automatically deduced. - One-parameter overload. - - @param function The member function to call - @param arg The argument to call the function with -***************************************************************************/ -template -template -void listener<_type_>::invoke(_member_fct_ function, _arg1_t_ arg) -{ - typename std::set::iterator it = instances.begin(); - typename std::set::iterator end = instances.end(); - for (; it != end; ++it) { - ((*it)->*function)(arg); - } -} - -/*************************************************************************** - Invokes a member function on all instances of an listener type. Template - parameters are meant to be automatically deduced. - - Two-parameters overload. - @param function The member function to call - @param arg1 The first argument to pass to the function - @param arg2 The second argument to pass to the function + @param args Arguments to pass ot the function. ***************************************************************************/ template -template -void listener<_type_>::invoke(_member_fct_ function, _arg1_t_ arg1, - _arg2_t_ arg2) +template +void listener<_type_>::invoke(_member_fct_ function, _args_ &&...args) { - typename std::set::iterator it = instances.begin(); - typename std::set::iterator end = instances.end(); - for (; it != end; ++it) { - ((*it)->*function)(arg1, arg2); + for (auto &instance : instances) { + (dynamic_cast(instance)->*function)( + std::forward<_args_>(args)...); } }