Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I thought it was a good idea to use this in my C++ projects:

class CRAIICall
{
public:
    typedef void (WINAPI * FnType)(HANDLE);

    CRAIICall(HANDLE h, FnType fun)
    : m_h(h), m_fun(fun) 
    {}

    ~CRAIICall() { 
        if (m_h) { m_fun(m_h); }
    }

private:
    HANDLE m_h;
    FnType m_fun;
};

and use it like this in a function:

HMODULE hSrClient = ::LoadLibraryW(L"srclient.dll");
CRAIICall(hSrClient, (CRAIICall::FnType)::FreeLibrary);

so that I can simply return from my function at any point, or throw exceptions.

Is it safe to cast a function pointer like this?

Other comments about this code are also appreciated.

share|improve this question
1  
In what way is this off-topic for code review? – Felix Dombek Sep 28 '12 at 15:29

1 Answer

up vote 4 down vote accepted

No it is not safe (It is undefined behavior):

The called function may put stuff on the stack (or somewhere else) as the return value. Expecting the caller to deal with it at their end. If the caller does not deal with the return value appropriately then you have undefined behavior (the returned object may have a destructor that needs to be called for example).

This line is probably not doing what you expect:

CRAIICall(hSrClient, (CRAIICall::FnType)::FreeLibrary);

This is creating a temporary object. Which goes out of scope at the end of the expression (the semicolon in this case). And thus the temporary objects destructor is getting called just after the constructor completes.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.