Re: Why does this cause "data abort" ?
Lisa Pearlson wrote:
class CMyWnd
{
[...]
BOOL RegisterWndClass(LPCTSTR lpszClassName, HINSTANCE hInstance,
WNDPROC wndProc) {
m_lpszClassName = lpszClassName;
m_hInstance = hInstance;
UnregisterWndClass();
WNDCLASS wc;
wc.style = CS_HREDRAW | CS_VREDRAW;
wc.lpfnWndProc = (WNDPROC) wndProc;
wc.cbClsExtra = 0;
wc.cbWndExtra = 0;
wc.hInstance = hInstance;
wc.hIcon = 0;
wc.hCursor = 0;
wc.hbrBackground = (HBRUSH) GetStockObject(HOLLOW_BRUSH);
wc.lpszMenuName = 0;
wc.lpszClassName = lpszClassName;
return (0 != RegisterClass(&wc));
}
I know that casting the result of GetStockObject() is necessary, but is it
also necessary for wndProc? Please remove all such casts, they only force
the compiler to accept conversions it otherwise would rightfully reject. If
they are necessary (like e.g. with the HINSTANCE/HANDLE parameter of
DllMain), at least properly document them!
virtual OnPaint(HDC hDC) {};
^ no!
// global vars. Do they need to be declared static???
static const g_szClassName[] = _T("MyClass");
static HINSTANCE g_hInstance = NULL;
static CMyWnd* pWnd = NULL;
If you don't need them to be global, don't do it. Please see your favourite
C++ book on what the 'static' means (there are three meanings!). Anyway, I
would say that the name of the windowclass is a private implementation
detail of the class, so it should be a class-static constant. The hInstance
is a bit problematic, as you only have it in DllMain() but later need it in
RegisterClass().
BOOL APIENTRY DllMain(HANDLE hModule, DWORD dwReason, LPVOID lpReserved)
{
switch (dwReason)
{
case DLL_PROCESS_ATTACH :
g_hInstance = (HINSTANCE) hModule;
pWnd = new CMyWnd();
pWnd->RegisterWndClass(g_szClassName, g_hInstance,
(WNDPROC) WndProc);
break;
case DLL_PROCESS_DETACH :
if (pWnd) {
pWnd->UnregisterWndClass();
delete pWnd;
}
break;
}
return TRUE;
}
Three things here:
1. There are restrictions what you can do inside DllMain, see the MSDN. I
would suggest you defer creation of a window to when it is used, e.g. like
this:
MyWindow& get_window()
{
// object is constructed when the function is first called
static MyWindow the_window;
return the_window;
}
and then inside the constructor check if the class has already been
registered and if not do that:
class MyWindow {
static size_t s_instances;
MyWindow() {
if(!s_instances)
Register();
++s_instances;
}
~MyWindow() {
--s_instances;
if(!s_instances)
Unregister();
}
static void Register();
static void Unregister();
};
In other words, both the window is created on demand and the windowclass is
registered on demand. Of course this requires some more work to work in a
multithreaded environment.
2. As above, you are casting a function pointer. This should not be
necessary!
// .def file has EXPORTS Initialize @ 240 NONAME
HWND Initialize(HWND hWndParent)
{
HICON hIcon = (HICON) LoadImage(
pWnd->Create(hWndParent, _T("My Window Title"));
return pWnd->m_hWnd;
}
Of course, you could to the initialisation here, too.
Uli