Passing a function to a template

Question

I'm writing a Win32Exception for an application I'm working on, the header file looks like this:

class Win32Exception : std::exception {
public:
    Win32Exception() = default;
    explicit Win32Exception(const std::wstring& message);
    explicit Win32Exception(const std::string& message);
    explicit Win32Exception(const char* message);
    virtual ~Win32Exception() throw() override = default;
    char const* what() const override;
    std::wstring message() const;
};

The implementation for the what function looks like this:

char const* Win32Exception::what() const
{
    DWORD flags = FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ALLOCATE_BUFFER;

    DWORD errorCode = GetLastError();

    DWORD systemLocale = MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US);

    LPSTR buffer = nullptr;

    DWORD length = FormatMessageA(flags, nullptr, errorCode, systemLocale, (LPSTR)&buffer, 0, nullptr);

    if(buffer != nullptr)
    {
        LocalFree(buffer);
    }
    else if (length == 0)
    {
        buffer = "Cannot get an error message. FormatMessage failed.";
    }

    return buffer;
}

The implementation for the message function looks like this:

std::wstring Win32Exception::message() const
{
    DWORD flags = FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ALLOCATE_BUFFER;

    DWORD errorCode = GetLastError();

    DWORD systemLocale = MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL);

    LPWSTR buffer = nullptr;

    DWORD length = FormatMessageW(flags, nullptr, errorCode, systemLocale, (LPWSTR)&buffer, 0, nullptr);

    if (buffer != nullptr)
    {
        LocalFree(buffer);
    }
    else if (length == 0)
    {
        buffer = L"Cannot get an error message. FormatMessage failed.";
    }

    return buffer;
}

Now, I would like to get rid of this duplication so I thought I can do something about it with a template, like this:

    template <typename TResult, DWORD (*formatMessageVersion)(DWORD, LPCVOID, DWORD, DWORD, TResult, DWORD, va_list*)>
TResult formatMessage(DWORD& systemLocale, DWORD& errorCode, TResult& fallbackMessage) const {
    DWORD flags = FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ALLOCATE_BUFFER;

    TResult buffer = nullptr;

    DWORD length = formatMessageVersion(flags, nullptr, errorCode, systemLocale, (TResult)&buffer, 0, nullptr);

    if (buffer != nullptr)
    {
        LocalFree(buffer);
    }
    else if (length == 0)
    {
        buffer = fallbackMessage;
    }

    return buffer;
}

In one of my attempts I tried to pass the function FormatMessageA/W as an argument to formatMessage through std::function but I'm not sure why it didn't work...

The way I thought I to use this is as follow:

char const* Win32Exception::what() const
{
    DWORD errorCode = GetLastError();

    DWORD systemLocale = MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US);

    return formatMessage<LPSTR, FormatMessageA>(systemLocale,
                                                errorCode,
                                                "Cannot get an error message. FormatMessage failed.");
}

std::wstring Win32Exception::message() const
{
    DWORD errorCode = GetLastError();

    DWORD systemLocale = MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL);

    return formatMessage<LPWSTR, FormatMessageW>(systemLocale,
                                                errorCode,
                                                L"Cannot get an error message. FormatMessage failed.");
}

But I'm getting bunch of errors:

Severity    Code    Description Project File    Line    Suppression State
Error   C2664   'TResult Yalla::Win32Exception::formatMessage<LPSTR,DWORD FormatMessageA(DWORD,LPCVOID,DWORD,DWORD,LPSTR,DWORD,va_list *)>(DWORD &,DWORD &,TResult &) const': cannot convert argument 3 from 'const char [51]' to 'LPSTR &' RunBox  d:\users\eyal\projects\code\yalla\core\src\runbox\win32\win32-exception.cpp 29  

Severity    Code    Description Project File    Line    Suppression State
Error   C2664   'TResult Yalla::Win32Exception::formatMessage<LPSTR,DWORD FormatMessageA(DWORD,LPCVOID,DWORD,DWORD,LPSTR,DWORD,va_list *)>(DWORD &,DWORD &,TResult &) const': cannot convert argument 3 from 'const char [51]' to 'LPSTR &' RunBox  d:\users\eyal\projects\code\yalla\core\src\runbox\win32\win32-exception.cpp 29  

Severity    Code    Description Project File    Line    Suppression State
Error   C2664   'TResult Yalla::Win32Exception::formatMessage<LPWSTR,DWORD FormatMessageW(DWORD,LPCVOID,DWORD,DWORD,LPWSTR,DWORD,va_list *)>(DWORD &,DWORD &,TResult &) const': cannot convert argument 3 from 'const wchar_t [51]' to 'LPWSTR &'   RunBox  d:\users\eyal\projects\code\yalla\core\src\runbox\win32\win32-exception.cpp 40  

Severity    Code    Description Project File    Line    Suppression State
Error   C2664   'TResult Yalla::Win32Exception::formatMessage<LPWSTR,DWORD FormatMessageW(DWORD,LPCVOID,DWORD,DWORD,LPWSTR,DWORD,va_list *)>(DWORD &,DWORD &,TResult &) const': cannot convert argument 3 from 'const wchar_t [51]' to 'LPWSTR &'   RunBox  d:\users\eyal\projects\code\yalla\core\src\runbox\win32\win32-exception.cpp 40  

Severity    Code    Description Project File    Line    Suppression State
Error (active)      no instance of function template "Yalla::Win32Exception::formatMessage" matches the argument list   RunBox  d:\Users\Eyal\Projects\Code\Yalla\core\src\runbox\win32\win32-exception.cpp 27  

Severity    Code    Description Project File    Line    Suppression State
Error (active)      no instance of function template "Yalla::Win32Exception::formatMessage" matches the argument list   RunBox  d:\Users\Eyal\Projects\Code\Yalla\core\src\runbox\win32\win32-exception.cpp 38

Bear in my mind that I'm relearning C++ and didn't touch it for many years so if you can tell me how to improve the code or do whatever that I'm trying to do in a better way, I'd be really glad to hear about it! ;)

Update: You can find the final solution at GitHub, thanks to everyone.

Unrelated to your question, your `what` and `message` return a deallocated buffer. Boom. — Konrad Rudolph, Mar 17 '16 at 14:17
You really shouldn't call GetLastError() in your `what()`. You do not know what happens between an exception generation and calling of `what()`. Instead, error code should be an argument to constructor. — SergeyA, Mar 17 '16 at 14:23
@KonradRudolph can you elaborate on this a bit? because based on my tests they seems to work. :) — iam3yal, Mar 17 '16 at 14:27
@SergeyA yeah! my plan is to add a ctor that takes an error code. — iam3yal, Mar 17 '16 at 14:28
@Eyal There’s not much to elaborate. You’re calling `LocalFree` on the buffer, and then you *subsequently* return it. The buffer no longer exists. — Konrad Rudolph, Mar 17 '16 at 14:34
`class Win32Exception : std::exception`... are you sure this shouldn't be public inheritance? — Christian Hackl, Mar 17 '16 at 16:49

score 2 · Accepted Answer · edited May 23 '17 at 12:24

Don't solve the duplication by templates. You already have a function, message(), that calls FormatMessageW and gets the message text. That's all you need. To implement what() you need an ANSI string. So call message() first then convert to ANSI.

In other words, implement what() by first calling message(). Then convert the std::wstring to std::string. Then return the const char* that c_str() yields. You will need to store the std::string in a member field though so that the pointer returned by c_str remains valid.

To perform the conversion, consult one of the many questions on the topic that has appeared here. For instance: How to convert wstring into string?

As it stands your current implementation of what() returns an invalid pointer because it calls LocalFree on the value that is subsequently returned. But if you follow my advice all of that code will go, so don't get too hung up on that.

Your error handling of FormatMessageW is wrong. Success, or otherwise, is indicated by the return value, as indicated by the documentation. A value of 0 means failure. That's what you need to be testing. I cannot stress strongly enough the importance of reading documentation very carefully. I'd say that >90% of the winapi questions that we see here have errors in the error handling.

What is more, you are again accessing buffer after it has been freed. The code should look more like this:

LPWSTR buffer;
DWORD length = FormatMessageW(flags, nullptr, errorCode, systemLocale, (LPWSTR)&buffer, 
    0, nullptr);
if (length == 0)
    return L"Cannot get an error message. FormatMessage failed.";

std::wstring retval = buffer;
LocalFree(buffer);
return retval;

You should allow the class to be passed the error code. Some error codes are returned directly by the API, e.g. registry APIs. Also, you can sometimes be confounded by the C++ runtime calling SetLastError before you have managed to get to the code that calls GetLastError.

Thank you so much! Yeah I thought about calling ```message``` from ```what``` but didn't know how exactly to do the conversion. — iam3yal, Mar 17 '16 at 14:34
That's a different question. http://stackoverflow.com/questions/4804298/how-to-convert-wstring-into-string — David Heffernan, Mar 17 '16 at 14:34

score 1 · Answer 2 · answered Mar 17 '16 at 14:13

The direct problem is described in your error message here:

[...] cannot convert argument 3 from 'const char [51]' to 'LPSTR &' [...]

You are passing a character literal to a function expecting a non-const reference. You cannot bind non-const references to a temporary, so the simple fix is simply to change the last argument type from TResult& to TResult const&. That should fix your problem.

Passing a function to a template

2 Answers2