#18 Invalid template code in Threading class

[Orcan Ogetbil]

dispatcher.h has invalid template code. When DBUS_HAS_RECURSIVE_MUTEX is #defined (which is the case for DBus >=0.95), the function pointers MutexFreeFn and MutexLockFn become of type void and the code is valid. See lines 232-235.
However, when a user #includes dispatcher.h directly or indirectly the macro DBUS_HAS_RECURSIVE_MUTEX is undefined. This makes the above function pointers defined of type bool and but then _init_threading function call in line 259 becomes invalid, as it passes function pointers to mutex_free and mutex_lock which or of type void.

In gcc7 this causes errors such as:

---

/usr/include/dbus-c++-1/dbus-c++/dispatcher.h: In static member function 'static void DBus::Threading<Mx, Cv="">::init()':
/usr/include/dbus-c++-1/dbus-c++/dispatcher.h:262:5: error: no matching function for call to '_init_threading(DBus::Mutex (&)(), void (&)(DBus::Mutex), void (&)(DBus::Mutex), void (&)(DBus::Mutex), DBus::CondVar (&)(), void (&)(DBus::CondVar), void (&)(DBus::CondVar, DBus::Mutex), bool (&)(DBus::CondVar, DBus::Mutex, int), void (&)(DBus::CondVar), void (&)(DBus::CondVar))'
);
^
/usr/include/dbus-c++-1/dbus-c++/dispatcher.h:247:13: note: candidate: void DBus::_init_threading()
void DXXAPI _init_threading();
^~~~~~~~~~~~~~~
/usr/include/dbus-c++-1/dbus-c++/dispatcher.h:247:13: note: candidate expects 0 arguments, 10 provided
/usr/include/dbus-c++-1/dbus-c++/dispatcher.h:249:13: note: candidate: void DBus::_init_threading(DBus::MutexNewFn, DBus::MutexFreeFn, DBus::MutexLockFn, DBus::MutexUnlockFn, DBus::CondVarNewFn, DBus::CondVarFreeFn, DBus::CondVarWaitFn, DBus::CondVarWaitTimeoutFn, DBus::CondVarWakeOneFn, DBus::CondVarWakeAllFn) <near match="">
void DXXAPI _init_threading(
^~~~~~~~~~~~~~~
/usr/include/dbus-c++-1/dbus-c++/dispatcher.h:249:13: note: conversion of argument 3 would be ill-formed

---

The proposed patch (provided by Jonathan Wakely) disables this part of the code altogether. Another approach would be to disable the #ifndef block in line 232.

[Orcan Ogetbil]

This issue came up when we were building the packages ffado and sflphone in Fedora, and the builds failed with gcc7. Refer to below links for the build.logs

https://koji.fedoraproject.org/koji/taskinfo?taskID=17749803
https://koji.fedoraproject.org/koji/taskinfo?taskID=17811337

[Jonathan Wakely]

There are several serious problems with the code in dispatcher.h

The typedefs MutexFreeFn and MutexLockFn depend on DBUS_HAS_RECURSIVE_MUTEX but as far as I can tell from the DBus documentation the functions that vary depending on the API version are the mutex lock and unlock functions, not the lock and free functions. So the wrong typedefs are affected by DBUS_HAS_RECURSIVE_MUTEX.

Threading<Mx, Cv>::mutex_lock always returns void, so is not valid as MutexLockFn when that returns bool. (Similarly for mutex_free or mutex_unlock ... whichever it is that's supposed to be conditional on the macro).

Some of the DBus threading API functions return dbus_bool_t which is a typedef for dbus_uint32_t, but the dbus-c++ ones return bool which leads to undefined behaviour because the return types are not compatible. And that's completely ignoring the fact that casting the void(*)(Mutex*) function pointers to void(*)(DbusMutexType*) and invoking them through that incorrect type is also undefined behaviour.

All of these issues could be fixed by using function pointers of the correct type, and casting between DBusMutexType* and Mutex* inside the functions (not by casting function pointers). Also, in 2017 it would probably make sense to drop all support for DBus versions before 1.0.0 when the API stabilised. But the simplest thing seems to be to remove all the broken code until it's proven to be needed.