Skip to content

Commit

Permalink
Fix and modernize listener<>
Browse files Browse the repository at this point in the history
Commit d1f50af 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<A>* 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.
  • Loading branch information
lmoureaux authored and jwrober committed Jan 8, 2024
1 parent e47bb57 commit 71c6e2f
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 64 deletions.
2 changes: 0 additions & 2 deletions client/chatline.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,10 @@ class QPaintEvent;
class QPainter;
class QPushButton;
class QUrl;
class chat_listener;

void set_chat_colors(const QHash<QString, QString> &colors);
QString apply_tags(QString str, const struct text_tag_list *tags,
QColor bg_color);
template <> std::set<chat_listener *> listener<chat_listener>::instances;
/***************************************************************************
Listener for chat. See listener<> for information about how to use it
***************************************************************************/
Expand Down
77 changes: 15 additions & 62 deletions client/listener.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#pragma once

#include <set>
#include <utility>

/***************************************************************************
Helper template to connect C and C++ code.
Expand Down Expand Up @@ -103,7 +104,7 @@ template <class _type_> class listener {

private:
// All instances of type_t that have called listen().
static std::set<type_t *> instances;
static std::set<listener<type_t> *> instances;

protected:
explicit listener();
Expand All @@ -112,21 +113,17 @@ template <class _type_> class listener {
void listen();

public:
template <class _member_fct_> static void invoke(_member_fct_ function);

template <class _member_fct_, class _arg1_t_>
static void invoke(_member_fct_ function, _arg1_t_ arg);

template <class _member_fct_, class _arg1_t_, class _arg2_t_>
static void invoke(_member_fct_ function, _arg1_t_ arg1, _arg2_t_ arg2);
template <class _member_fct_, class... _args_>
static void invoke(_member_fct_ function, _args_ &&...args);
};

/***************************************************************************
Macro to declare the static data needed by listener<> classes
***************************************************************************/
#define FC_CPP_DECLARE_LISTENER(_type_) \
template <> \
std::set<_type_ *> listener<_type_>::instances = std::set<_type_ *>();
std::set<listener<_type_> *> listener<_type_>::instances = \
std::set<listener<_type_> *>();

/***************************************************************************
Constructor
Expand All @@ -140,74 +137,30 @@ template <class _type_> 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<type_t *>(this));
instances.insert(this);
}

/***************************************************************************
Destructor
***************************************************************************/
template <class _type_> listener<_type_>::~listener()
{
instances.erase(dynamic_cast<type_t *>(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 <class _type_>
template <class _member_fct_>
void listener<_type_>::invoke(_member_fct_ function)
{
typename std::set<type_t *>::iterator it = instances.begin();
typename std::set<type_t *>::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 <class _type_>
template <class _member_fct_, class _arg1_t_>
void listener<_type_>::invoke(_member_fct_ function, _arg1_t_ arg)
{
typename std::set<type_t *>::iterator it = instances.begin();
typename std::set<type_t *>::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 <class _type_>
template <class _member_fct_, class _arg1_t_, class _arg2_t_>
void listener<_type_>::invoke(_member_fct_ function, _arg1_t_ arg1,
_arg2_t_ arg2)
template <class _member_fct_, class... _args_>
void listener<_type_>::invoke(_member_fct_ function, _args_ &&...args)
{
typename std::set<type_t *>::iterator it = instances.begin();
typename std::set<type_t *>::iterator end = instances.end();
for (; it != end; ++it) {
((*it)->*function)(arg1, arg2);
for (auto &instance : instances) {
(dynamic_cast<type_t *>(instance)->*function)(
std::forward<_args_>(args)...);
}
}

0 comments on commit 71c6e2f

Please sign in to comment.