Windows DLL Module Class

Question

I am doing a project where I use DLL's as run-time modules or plugins. To do this, I use the Windows LoadLibrary() and FreeLibrary() API calls to call two functions exported by the DLL. I decided to make a class so dealing with the DLL's would be easier. Is there anything I can add to my class to make it better, is there anything I'm doing wrong? Feel free to criticize anything you want.

SaModule.h

---

#ifndef SAMODULE_H__
#define SAMODULE_H__

#include <Windows.h>
#include <string>
#include <exception>
#include "WinException.h"

class SaModule
{
private:

    typedef const char* (*LPFNGETNAME)();
    typedef bool (*LPFNDOSCAN)(std::string&, std::string&);
    HINSTANCE hInstance;
    std::string path;

public:

    SaModule();
    SaModule(const std::string& path);
    ~SaModule();

    void Load(const std::string& path);
    void Unload();

    LPFNDOSCAN DoScan;
    LPFNGETNAME GetName;

    HINSTANCE GetHandle() const;
    operator HINSTANCE() const;

};

#endif//SAMODULE_H__

SaModule.cpp

---

#include "SaModule.h"
#include "WinException.h"
#include <sstream>
#include <iomanip>

SaModule::SaModule(): path(), hInstance(NULL)
{
    //Nothing...
}

SaModule::SaModule(const std::string& path): path(path), hInstance(NULL)
{
    Load(path);
}

SaModule::~SaModule()
{
    Unload();
}

void SaModule::Load(const std::string& path)
{

    this->path = path;

    if( NULL == (hInstance=LoadLibrary(path.c_str())) )
        throw WinException();

    if( NULL == (GetName=reinterpret_cast<LPFNGETNAME>(GetProcAddress(hInstance,"GetName"))) )
        throw WinException();

    if( NULL == (DoScan=reinterpret_cast<LPFNDOSCAN>(GetProcAddress(hInstance, "DoScan"))) )
        throw WinException();

}

void SaModule::Unload()
{
    FreeLibrary(hInstance);
}

HINSTANCE SaModule::GetHandle() const
{
    return hInstance;
}

SaModule::operator HINSTANCE() const
{
    return hInstance;
}

WinException.h

---

#ifndef WINEXCEPTION_H__
#define WINEXCEPTION_H__

#include <Windows.h>
#include <exception>
#include <string>

class WinException: public std::exception
{
private:

    DWORD dwError;
    std::string errMsg;

public:

    WinException();
    const char* what() const;


};

#endif//WINEXCEPTION_H__

WinException.cpp

---

#include "WinException.h"
#include <sstream>
#include <iomanip>

WinException::WinException()
{

    std::ostringstream ss;
    dwError = GetLastError();   
    LPVOID lpMsgBuf = NULL;

    FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER|FORMAT_MESSAGE_FROM_SYSTEM|FORMAT_MESSAGE_IGNORE_INSERTS,
        NULL, dwError, MAKELANGID(LANG_NEUTRAL,SUBLANG_DEFAULT), reinterpret_cast<LPTSTR>(&lpMsgBuf), 0, NULL);

    ss << "Error: 0x" << std::hex << std::setfill('0') << std::setw(8) << dwError << std::endl
        << reinterpret_cast<LPTSTR>(lpMsgBuf);

    errMsg = ss.str();

    LocalFree(lpMsgBuf);

}

const char* WinException::what() const
{
    return errMsg.c_str();
}

Example Usage

---

#include <iostream>
#include "SaModule.h"

int main(int argc, char* argv[])
{

    try
    {
        SaModule mod("some.sa.dll");
        std::cout << "Loaded Module: " << mod.GetName() << std::endl;
    }
    catch( WinException& ex )
    {
        std::cout << ex.what() << std::endl;
    }

    return 0;

}

Answer 1

Just a couple of quick thoughts.

1. DoScan and GetName probably should be initialised to NULL as well.
2. Load doesn't call Unload for any previous loaded dlls. If Load should be called once only per SaModule, that should be detected.
3. A more informative exception could be thrown. "...during attempted insertion of plugin xyz", for example.
4. Beware of function name decoration, if that could be an issue.

Answer 2

First thing that pops out at me:

if( NULL == (hInstance=LoadLibrary(path.c_str())) )

When programming for Windows, you should almost never want to use the "A"/"ANSI" versions of functions. This is legacy from Win9x and you should be using Unicode. Windows is not like Unix where the C library can assume UTF-8 and everything can keep chugging along with the full set of Unicode chars accesible: by restricting yourself to the "multi-byte" encodings you will (1) see inconsistent behavior depending on what the user sets their language to and (2) not be able to access some Unicode filenames that the user may have on their filesystem. You should define the UNICODE and _UNICODE macros and use WCHAR instead of 8-bit chars. (Since it seems you want to use the C++ classes I believe wstring works instead of string here.)

Second:

The WinException class looks weird to me. Not all Windows APIs use GetLastError - some of them may return HRESULT or do something else. From that perspective you have not captured the full generality of Windows errors.

I would also say, perhaps controversially since this is not the path everybody has taken, that wrapping Win32 errors with exceptions is a bad idea. There are some Win32 errors that are not error conditions at all, for example FindNextFile will fail with ERROR_NO_MORE_ITEMS when you reach the end of a directory, or I/O will fail with ERROR_IO_PENDING to indicate that the I/O is happening asynchronously. A Win32 program will occasionally have to react to such errors and I would say the way you've done it loses some of that information, or makes it less convenient to work with.

But maybe it's OK for your use. YMMV.

Answer 3

As MikeC suggested, the WinException class could be replaced by std::system_error. This original came from the boost::system library and was included into the C++0x specification with a few changes (mainly some of the names).

Example for throw:

throw std::system_error(std::error_code(::GetLastError(), 
    std::system_category()), "SaModule::Load()");

Example for catch:

//...
catch( std::system_error& se )
{
    std::cout << "Error: " << se.code().value() << "\n";
    std::cout << "Message: " << se.code().message() << "\n";
    std::cout << "What: " << se.what() << "\n";
}

Example output for ERROR_FILE_NOT_FOUND:

Error: 2
Message: No such file or directory
What: SaModule::Load()