views:

154

answers:

4

I have a worker thread, which holds a list of 'Thread Actions', and works through them as an when.

template <class T> class ThreadAction
{
public:

  typedef void (T::*action)();

  ThreadAction(T* t, action f) :
    func(f),obj(t) {}
  void operator()() { (obj->*func)(); }

  void (T::*func)();
  T* obj;

};

It's normally called like this

myActionThread->addAction(
    new ThreadAction<TheirClass>(this, &TheirClass::enable)
);

Which worked fine until

 void TheirClass::enable()

was changed to

 bool TheirClass::enable()

Sadly we can't change it back again because other stuff needs the new format, (and overloads can't differ by return type alone).

I did try

myActionThread->addAction( 
    new ThreadAction<TheirClass>(this, 
        reinterpret_cast<void(TheirClass::*)>(&TheirClass::enable)
    )
);

Which appears to work just fine, but I'm not certain that reinterpreting a function pointer like this is 'defined' behaviour, can somebody please advise?

+8  A: 

This is definitely not supported behavior, and can potentially cause your program to crash.

Basically, you need to make a wrapper for TheirClass::enable() that will have the proper return type. A simple one-liner will suffice:

public:
    void enableWrapper() { enable(); };

Then call:

myActionThread->addAction(
    new ThreadAction<TheirClass>(this, &TheirClass::enableWrapper)
);

If you can't modify TheirClass directly, then create a simple subclass or helper class that implements the wrapper.

JSBangs
Reinterpreting member function pointers is particularly bad because their size can vary depending on the class: http://blogs.msdn.com/oldnewthing/archive/2004/02/09/70002.aspx
bdonlan
+2  A: 

Err, from what I've understood, you're casting from a method that returns a bool to a method that returns void ?

This may be dangerous, depending on the calling/returning convention in use. You might forget to pop the return value - or override the value of a register with the return value.

Aurélien Vallée
+1 Good point about overwriting a register with the return value.
Martin B
+2  A: 

Not a good idea. Consider adding an additional template parameter for return type:

template <class T, typename RetType> class ThreadAction
{
public:
 typedef RetType (T::*action)();
 ThreadAction(T* t, action f) :
   func(f),obj(t) {}

 RetType operator()() { return (obj->*func)(); }
 RetType (T::*func)();
 T* obj;
};

This is an application of return void.

David Joyner
In typedef, void need to change to RetType
Sumant
Fixed it, thanks!
David Joyner
+1  A: 

I generally find that when the question is of the form "Is ___ a good idea?" the answer is almost invariably "NO!"

This is probably true without context.

Brian Postow
SO do allow you to answer your own questions.... :-)
Chris Huang-Leaver