Why does this cause "data abort" ?

Hi imagine construction like below:

class CSomeWnd
{
public:
int m_hWnd;
virtual LRESULT SomeWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM
lParam);
}

CSomeWnd* g_pSomeWnd = new CSomeWnd();
g_pSomeWnd->CreateWindow( ... );
...
LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
{
return g_pSomeWnd->SomeWndProc(hWnd,msg,lParam,wParam);
}


Since I can not register window class with a non-static windows procedure, I
can not use a non-static member functions.
So above the global static GlobalWndProc 'relays' the message to non-static
SomeWndProc, so that SomeWndProc can access member variables easily.

However, the above construction causes data abort exceptions. I don't
understand why, but it's resolved by something like this:

CSomeWnd* g_pSomeWnd = new CSomeWnd();
g_pSomeWnd->CreateWindow( ... );
SetWindowLong(g_pSomeWnd->GetSafeHwnd(), GWL_USERDATA, (LONG) this);

LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
{
CSomeWnd* pWnd = (CSomeWnd*) GetWindowLong(hWnd, GWL_USERDATA);
return pWnd->SomeWndProc(hWnd,msg,lParam,wParam);
}

Can someone explain why this is?
And.. is there a better way to implement the above? some pattern?

Thanks,
Lisa

Giovanni
Wed Oct 24 01:04:49 PDT 2007


"Lisa Pearlson" <no@spam.plz> ha scritto nel messaggio
news:eZNXTQeFIHA.1316@TK2MSFTNGP02.phx.gbl...

> class CSomeWnd
> {
> public:
> int m_hWnd;
> virtual LRESULT SomeWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM
> lParam);
> }
>
> CSomeWnd* g_pSomeWnd = new CSomeWnd();
> g_pSomeWnd->CreateWindow( ... );
> ...
> LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
> {
> return g_pSomeWnd->SomeWndProc(hWnd,msg,lParam,wParam);
> }
[...]
> However, the above construction causes data abort exceptions.

I don't know what *exactly* "data abort exceptions" are...

However, I can't see anything wrong in the above code. Maybe the exception
is thrown somewhere else...

Frankly speaking, I don't like using the global instance as shown above. I
prefer the following code you posted:

> understand why, but it's resolved by something like this:
>
> CSomeWnd* g_pSomeWnd = new CSomeWnd();
> g_pSomeWnd->CreateWindow( ... );
> SetWindowLong(g_pSomeWnd->GetSafeHwnd(), GWL_USERDATA, (LONG) this);
>
> LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
> {
> CSomeWnd* pWnd = (CSomeWnd*) GetWindowLong(hWnd, GWL_USERDATA);
> return pWnd->SomeWndProc(hWnd,msg,lParam,wParam);
> }

If you don't want to use GWL_USERDATA, you may consider also Window
Properties technique (i.e. use the SetProp and GetProp Win32 APIs to store
the class instance pointer "this", associating it to a string property); or
you can use extra window bytes (the "cbWndExtra" field in the WNDCLASSEX
structure) to store the "this" pointer (e.g. set windowClass.cbWndExtra =
sizeof( YourClass * ), and use SetWindowLong and GetWindowLong using 0-byte
offset to set and get the "this" instance pointer).

There are also more advanced techniques, like using assembly language
"thunks". This is done by ATL/WTL, IIRC.

However, if you use ATL (or MFC) you can just derive your class from the
base window class (or some other window class), using just C++ inheritance.

Giovanni

Ulrich
Wed Oct 24 01:08:29 PDT 2007

Lisa Pearlson wrote:
> Hi imagine construction like below:
>
> class CSomeWnd
> {
> public:
> int m_hWnd;
> virtual LRESULT SomeWndProc(HWND hWnd, MSG msg,
> WPARAM wParam, LPARAM lParam);
> }
>

How does that which you are trying look like? The reason I'm asking is that
this obviously lacks a semicolon so it probably is not the code that is
compiled and executed, making it hard to guess what's going on.

One other thing here: the member m_hWnd should probably be a HWND. Further,
I guess that the hWnd passed to SomeWndProc is the same as the one stored
in the member, right? In that case, I'd suggest dropping either of them.

Further, I'd drop the 'C' in front of the classname. Using such things to
separate different class libraries is obsolete in the presence of real C++
namespaces. Also, it makes it look like somehow related to the MFC.

> CSomeWnd* g_pSomeWnd = new CSomeWnd();
> g_pSomeWnd->CreateWindow( ... );
> ...
> LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
> {
> return g_pSomeWnd->SomeWndProc(hWnd,msg,lParam,wParam);
> }

Again, what does the code look like? In the first line, you seem to
initialise a local pointer named g_pSomeWnd but in the function you use the
global one.

Just for your info: most class libraries I have seen simply maintain a map
between the HWND and a pointer to the window class. Of course, using
GetWindowLong() is another possible way to achieve that.

> Since I can not register window class with a non-static windows procedure,
> I can not use a non-static member functions.
> So above the global static GlobalWndProc 'relays' the message to
> non-static SomeWndProc, so that SomeWndProc can access member variables
> easily.
>
> However, the above construction causes data abort exceptions. I don't
> understand why, but it's resolved by something like this:
>
> CSomeWnd* g_pSomeWnd = new CSomeWnd();
> g_pSomeWnd->CreateWindow( ... );
> SetWindowLong(g_pSomeWnd->GetSafeHwnd(), GWL_USERDATA, (LONG) this);
>
> LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
> {
> CSomeWnd* pWnd = (CSomeWnd*) GetWindowLong(hWnd, GWL_USERDATA);
> return pWnd->SomeWndProc(hWnd,msg,lParam,wParam);
> }
>
> Can someone explain why this is?

Using GetWindowLong()/SetWindowLong(), you simply retrieve/attach a value of
type LONG from a window handle. This value can also be a pointer (though
you should use GetWindowLongPtr() then, see MSDN) to the wrapper class that
handles the windowmessage.

Note: use reinterpret_cast to convert between a pointer and a LONG.
Preferably forget about the fact that C-style casts exist at all.

Uli

Lisa
Wed Oct 24 05:49:21 PDT 2007

Ulrich,

Thank you for some interesting pointers. My background is indeed mostly MFC,
but am doing embedded CE development now.
The code I wrote is indeed not original code, but after long searching, I
found "data abort" (it's an exception raised by ARM cpu's, when out of
bounds memory is accessed) is caused by the call to CreateWindow. The window
gets created and windows handle returned, but there's nothing there that can
go wrong, other than the initial call to WM_CREATE I think. I am not
handling WM_CREATE, or just returning 0, so there's no code there that can
cause the problem.

The only other thing that could possibly be related, is fact that it is
inside a DLL like so:

// sample class
class CMyWnd
{
public:
CMyWnd() :
m_hWnd(NULL), m_lpszClassName(NULL),
m_hInstance(NULL), m_hIcon(NULL) {};
virtual ~CMyWnd(){
if (m_hIcon) DestroyIcon(m_hIcon);
};

HWND m_hWnd;
HINSTANCE m_hInstance;
LPTSTR m_lpszClassName;
HICON m_hIcon;

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));
}
void UnregisterWndClass() {
UnregisterClass(m_lpszClassName, m_hInstance);
}

BOOL SetIcon(UINT nRescID) {
m_hIcon = (HICON) LoadImage(m_hInstance,
MAKEINTRESOURCE(nRescID), IMAGE_ICON, 16,
16, LR_DEFAULTCOLOR);
return (NULL != m_hIcon);
}

BOOL Create(HWND hParentWnd, LPCTSTR lpszWindowName, DWORD
dwStyle=WS_VISIBLE|WS_CHILD) {
m_hWnd = CreateWindow(m_lpszClassName, m_lpszWindowName,
dwStyle,
0, 0, 0, 0, hParentWnd, NULL,
m_hInstance, NULL);
return (NULL != m_hWnd);
}

virtual OnPaint(HDC hDC) {};
virtual OnCreate(LPCREATESTRUCT lpcs) { return 0; };
virtual OnMessage(MSG uMsg, WPARAM wParam, LPARAM lParam) {
switch (uMsg)
{
case WM_CREATE:
{
LPCREATESTRUCT lpcs = (LPCREATESTRUCT) lParam;
return OnCreate(lpcs);
}
case WM_PAINT:
{
PAINTSTRUCT ps;
HDC hDC = BeginPaint(m_hWnd, &ps);
OnPaint(hDC);
EndPaint(m_hWnd, &ps);
return 0;
}
case WM_DESTROY:
PostQuitMessage(0);
return 0;
}
return DefWindowProc(m_hWnd, uMsg, wParam, lParam);
}
}

// global vars. Do they need to be declared static???
static const g_szClassName[] = _T("MyClass");
static HINSTANCE g_hInstance = NULL;
static CMyWnd* pWnd = NULL;

LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
return pWnd->OnMessage(uMsg, wParam, lParam);
}

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;
}

// .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;
}


This is a plugin, where Initialize gets called by an application, via
ordinal, passing it its own hwnd.
The above is causing a "data abort" exception in coredll.

It doesn't cause exception when I use SetWindowLong(m_hWnd, GWL_USERDATA,
(LONG) this); in CMyWnd::Create() after CreateWindow, and then CMyWnd* pWnd
= reinterpret_cast<CMyWnd*>(GetWindowLong(m_hWnd, GWL_USERDATA)); in WndProc
and then pWnd->OnMessage(...).

But I don't understand why.. plus, the latter causes all kinds of other
exceptions elsewhere in my code, but that's of later concern.
I haven't compiled above code, just typed it here, but it's really all
that's going on in my code.

I'm not sure how the host application is using and calling my dll exactely..
maybe DLL loaded in a different thread than Initialize being called or some
other way that might cause issues, and why first way causes problems but is
resolved with SetWindowLong method.

Any clues?



"Ulrich Eckhardt" <eckhardt@satorlaser.com> wrote in message
news:35b3v4-3q6.ln1@satorlaser.homedns.org...
> Lisa Pearlson wrote:
>> Hi imagine construction like below:
>>
>> class CSomeWnd
>> {
>> public:
>> int m_hWnd;
>> virtual LRESULT SomeWndProc(HWND hWnd, MSG msg,
>> WPARAM wParam, LPARAM lParam);
>> }
>>
>
> How does that which you are trying look like? The reason I'm asking is
> that
> this obviously lacks a semicolon so it probably is not the code that is
> compiled and executed, making it hard to guess what's going on.
>
> One other thing here: the member m_hWnd should probably be a HWND.
> Further,
> I guess that the hWnd passed to SomeWndProc is the same as the one stored
> in the member, right? In that case, I'd suggest dropping either of them.
>
> Further, I'd drop the 'C' in front of the classname. Using such things to
> separate different class libraries is obsolete in the presence of real C++
> namespaces. Also, it makes it look like somehow related to the MFC.
>
>> CSomeWnd* g_pSomeWnd = new CSomeWnd();
>> g_pSomeWnd->CreateWindow( ... );
>> ...
>> LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
>> {
>> return g_pSomeWnd->SomeWndProc(hWnd,msg,lParam,wParam);
>> }
>
> Again, what does the code look like? In the first line, you seem to
> initialise a local pointer named g_pSomeWnd but in the function you use
> the
> global one.
>
> Just for your info: most class libraries I have seen simply maintain a map
> between the HWND and a pointer to the window class. Of course, using
> GetWindowLong() is another possible way to achieve that.
>
>> Since I can not register window class with a non-static windows
>> procedure,
>> I can not use a non-static member functions.
>> So above the global static GlobalWndProc 'relays' the message to
>> non-static SomeWndProc, so that SomeWndProc can access member variables
>> easily.
>>
>> However, the above construction causes data abort exceptions. I don't
>> understand why, but it's resolved by something like this:
>>
>> CSomeWnd* g_pSomeWnd = new CSomeWnd();
>> g_pSomeWnd->CreateWindow( ... );
>> SetWindowLong(g_pSomeWnd->GetSafeHwnd(), GWL_USERDATA, (LONG) this);
>>
>> LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
>> {
>> CSomeWnd* pWnd = (CSomeWnd*) GetWindowLong(hWnd, GWL_USERDATA);
>> return pWnd->SomeWndProc(hWnd,msg,lParam,wParam);
>> }
>>
>> Can someone explain why this is?
>
> Using GetWindowLong()/SetWindowLong(), you simply retrieve/attach a value
> of
> type LONG from a window handle. This value can also be a pointer (though
> you should use GetWindowLongPtr() then, see MSDN) to the wrapper class
> that
> handles the windowmessage.
>
> Note: use reinterpret_cast to convert between a pointer and a LONG.
> Preferably forget about the fact that C-style casts exist at all.
>
> Uli
>

Lisa
Wed Oct 24 06:22:38 PDT 2007

Oops, I forgot to finish my LoadImage function.. it for some reason FAILS
too even though there is icon resource with 16x16 icon.
GetLastError returns something like "no image by that name found in image
file"..

// .def file has EXPORTS Initialize @ 240 NONAME
HWND Initialize(HWND hWndParent)
{
m_hIcon = (HICON)LoadImage(m_hInstance,
MAKEINTRESOURCE(IDI_ICON), IMAGE_ICON, 16, 16,
LR_DEFAULTCOLOR);

pWnd->Create(hWndParent, _T("My Window Title"));
return pWnd->m_hWnd;
}

But that's not my main issue right now.


"Lisa Pearlson" <no@spam.plz> wrote in message
news:uyyHzvjFIHA.1204@TK2MSFTNGP03.phx.gbl...
> Ulrich,
>
> Thank you for some interesting pointers. My background is indeed mostly
> MFC, but am doing embedded CE development now.
> The code I wrote is indeed not original code, but after long searching, I
> found "data abort" (it's an exception raised by ARM cpu's, when out of
> bounds memory is accessed) is caused by the call to CreateWindow. The
> window gets created and windows handle returned, but there's nothing there
> that can go wrong, other than the initial call to WM_CREATE I think. I am
> not handling WM_CREATE, or just returning 0, so there's no code there that
> can cause the problem.
>
> The only other thing that could possibly be related, is fact that it is
> inside a DLL like so:
>
> // sample class
> class CMyWnd
> {
> public:
> CMyWnd() :
> m_hWnd(NULL), m_lpszClassName(NULL),
> m_hInstance(NULL), m_hIcon(NULL) {};
> virtual ~CMyWnd(){
> if (m_hIcon) DestroyIcon(m_hIcon);
> };
>
> HWND m_hWnd;
> HINSTANCE m_hInstance;
> LPTSTR m_lpszClassName;
> HICON m_hIcon;
>
> 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));
> }
> void UnregisterWndClass() {
> UnregisterClass(m_lpszClassName, m_hInstance);
> }
>
> BOOL SetIcon(UINT nRescID) {
> m_hIcon = (HICON) LoadImage(m_hInstance,
> MAKEINTRESOURCE(nRescID), IMAGE_ICON, 16,
> 16, LR_DEFAULTCOLOR);
> return (NULL != m_hIcon);
> }
>
> BOOL Create(HWND hParentWnd, LPCTSTR lpszWindowName, DWORD
> dwStyle=WS_VISIBLE|WS_CHILD) {
> m_hWnd = CreateWindow(m_lpszClassName, m_lpszWindowName,
> dwStyle,
> 0, 0, 0, 0, hParentWnd, NULL,
> m_hInstance, NULL);
> return (NULL != m_hWnd);
> }
>
> virtual OnPaint(HDC hDC) {};
> virtual OnCreate(LPCREATESTRUCT lpcs) { return 0; };
> virtual OnMessage(MSG uMsg, WPARAM wParam, LPARAM lParam) {
> switch (uMsg)
> {
> case WM_CREATE:
> {
> LPCREATESTRUCT lpcs = (LPCREATESTRUCT) lParam;
> return OnCreate(lpcs);
> }
> case WM_PAINT:
> {
> PAINTSTRUCT ps;
> HDC hDC = BeginPaint(m_hWnd, &ps);
> OnPaint(hDC);
> EndPaint(m_hWnd, &ps);
> return 0;
> }
> case WM_DESTROY:
> PostQuitMessage(0);
> return 0;
> }
> return DefWindowProc(m_hWnd, uMsg, wParam, lParam);
> }
> }
>
> // global vars. Do they need to be declared static???
> static const g_szClassName[] = _T("MyClass");
> static HINSTANCE g_hInstance = NULL;
> static CMyWnd* pWnd = NULL;
>
> LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM
> lParam)
> {
> return pWnd->OnMessage(uMsg, wParam, lParam);
> }
>
> 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;
> }
>
> // .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;
> }
>
>
> This is a plugin, where Initialize gets called by an application, via
> ordinal, passing it its own hwnd.
> The above is causing a "data abort" exception in coredll.
>
> It doesn't cause exception when I use SetWindowLong(m_hWnd, GWL_USERDATA,
> (LONG) this); in CMyWnd::Create() after CreateWindow, and then CMyWnd*
> pWnd = reinterpret_cast<CMyWnd*>(GetWindowLong(m_hWnd, GWL_USERDATA)); in
> WndProc and then pWnd->OnMessage(...).
>
> But I don't understand why.. plus, the latter causes all kinds of other
> exceptions elsewhere in my code, but that's of later concern.
> I haven't compiled above code, just typed it here, but it's really all
> that's going on in my code.
>
> I'm not sure how the host application is using and calling my dll
> exactely.. maybe DLL loaded in a different thread than Initialize being
> called or some other way that might cause issues, and why first way causes
> problems but is resolved with SetWindowLong method.
>
> Any clues?
>
>
>
> "Ulrich Eckhardt" <eckhardt@satorlaser.com> wrote in message
> news:35b3v4-3q6.ln1@satorlaser.homedns.org...
>> Lisa Pearlson wrote:
>>> Hi imagine construction like below:
>>>
>>> class CSomeWnd
>>> {
>>> public:
>>> int m_hWnd;
>>> virtual LRESULT SomeWndProc(HWND hWnd, MSG msg,
>>> WPARAM wParam, LPARAM lParam);
>>> }
>>>
>>
>> How does that which you are trying look like? The reason I'm asking is
>> that
>> this obviously lacks a semicolon so it probably is not the code that is
>> compiled and executed, making it hard to guess what's going on.
>>
>> One other thing here: the member m_hWnd should probably be a HWND.
>> Further,
>> I guess that the hWnd passed to SomeWndProc is the same as the one stored
>> in the member, right? In that case, I'd suggest dropping either of them.
>>
>> Further, I'd drop the 'C' in front of the classname. Using such things to
>> separate different class libraries is obsolete in the presence of real
>> C++
>> namespaces. Also, it makes it look like somehow related to the MFC.
>>
>>> CSomeWnd* g_pSomeWnd = new CSomeWnd();
>>> g_pSomeWnd->CreateWindow( ... );
>>> ...
>>> LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
>>> {
>>> return g_pSomeWnd->SomeWndProc(hWnd,msg,lParam,wParam);
>>> }
>>
>> Again, what does the code look like? In the first line, you seem to
>> initialise a local pointer named g_pSomeWnd but in the function you use
>> the
>> global one.
>>
>> Just for your info: most class libraries I have seen simply maintain a
>> map
>> between the HWND and a pointer to the window class. Of course, using
>> GetWindowLong() is another possible way to achieve that.
>>
>>> Since I can not register window class with a non-static windows
>>> procedure,
>>> I can not use a non-static member functions.
>>> So above the global static GlobalWndProc 'relays' the message to
>>> non-static SomeWndProc, so that SomeWndProc can access member variables
>>> easily.
>>>
>>> However, the above construction causes data abort exceptions. I don't
>>> understand why, but it's resolved by something like this:
>>>
>>> CSomeWnd* g_pSomeWnd = new CSomeWnd();
>>> g_pSomeWnd->CreateWindow( ... );
>>> SetWindowLong(g_pSomeWnd->GetSafeHwnd(), GWL_USERDATA, (LONG) this);
>>>
>>> LRESULT GlobalWndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
>>> {
>>> CSomeWnd* pWnd = (CSomeWnd*) GetWindowLong(hWnd, GWL_USERDATA);
>>> return pWnd->SomeWndProc(hWnd,msg,lParam,wParam);
>>> }
>>>
>>> Can someone explain why this is?
>>
>> Using GetWindowLong()/SetWindowLong(), you simply retrieve/attach a value
>> of
>> type LONG from a window handle. This value can also be a pointer (though
>> you should use GetWindowLongPtr() then, see MSDN) to the wrapper class
>> that
>> handles the windowmessage.
>>
>> Note: use reinterpret_cast to convert between a pointer and a LONG.
>> Preferably forget about the fact that C-style casts exist at all.
>>
>> Uli
>>
>
>

Ulrich
Wed Oct 24 06:38:38 PDT 2007

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

Giovanni
Wed Oct 24 07:14:45 PDT 2007


"Lisa Pearlson" <no@spam.plz> ha scritto nel messaggio
news:uyyHzvjFIHA.1204@TK2MSFTNGP03.phx.gbl...

> // sample class
> class CMyWnd
> {
> public:
> CMyWnd() :
> m_hWnd(NULL), m_lpszClassName(NULL),
> m_hInstance(NULL), m_hIcon(NULL) {};

> LPTSTR m_lpszClassName;

I don't like this data member here.
I would prefer storing the class name into a buffer using deep-copy (e.g.
TCHAR m_className[ 200 ];) or better use a robust string class, like
CString), e.g.:

CString m_className;

And adjust here:

> BOOL RegisterWndClass(LPCTSTR lpszClassName, HINSTANCE hInstance,
> WNDPROC wndProc) {
> m_lpszClassName = lpszClassName;

// Deep copy
m_className = lpszClassName;

> void UnregisterWndClass() {
> UnregisterClass(m_lpszClassName, m_hInstance);
> }

I'm not sure if the class should be unregistered here...
I would *not* unregister the window class.

http://blogs.msdn.com/oldnewthing/archive/2007/02/12/1661754.aspx


> // global vars. Do they need to be declared static???
> static const g_szClassName[] = _T("MyClass");
> static HINSTANCE g_hInstance = NULL;
> static CMyWnd* pWnd = NULL;


> LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM
> lParam)
> {
> return pWnd->OnMessage(uMsg, wParam, lParam);

I would check if pWnd is not null here, e.g. using an assert:

_ASSERTE( pWnd != NULL );


> case DLL_PROCESS_DETACH :
> if (pWnd) {
> pWnd->UnregisterWndClass();
> delete pWnd;

Try not unregistering here, and after delete pWnd, set also pWnd = NULL
(just to avoid the use of a uninitialized pointer).

> // .def file has EXPORTS Initialize @ 240 NONAME
> HWND Initialize(HWND hWndParent)

Should this be __stdcall?
Maybe there is some stack corruption due to uncorrect calling conventions?

However, I would not use this kind of "mixed C - C++" code; I would go for a
more robust C++ code.
I would use ATL/WTL (I don't know embedded development, maybe you have
memory limits so you can't use MFC? But in that case I think you could use
ATL, because ATL is a very *thin* object-oriented wrapper to Win32 APIs -
and you can have the benefit of robust object-oriented code).

Giovanni

Scott
Wed Oct 24 08:02:04 PDT 2007

"Lisa Pearlson" <no@spam.plz> wrote:
> BOOL RegisterWndClass(LPCTSTR lpszClassName, HINSTANCE hInstance,
>WNDPROC wndProc) {
> m_lpszClassName = lpszClassName;

It's generally a bad idea to keep this sort of pointer around. In
your case it'll work, but you have no guarantee that the caller isn't
passing you a pointer to a variable on the heap, or a bit of memory
they're about to deallocate.

>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)

You'll want to avoid doing pretty much anything interesting in
DllMain(). See
http://blogs.msdn.com/oldnewthing/archive/2004/01/27/63401.aspx
for the details.

>// .def file has EXPORTS Initialize @ 240 NONAME
>HWND Initialize(HWND hWndParent)
>{
> HICON hIcon = (HICON) LoadImage(
> pWnd->Create(hWndParent, _T("My Window Title"));

What's the LoadImage call supposed to be doing here? I wouldn't be
surprised to learn that this is the cause of your problem, but that's
just a guess.

--
--------- Scott Seligman <scott at <firstname> and michelle dot net> ---------
Anger is the most useless emotion, .. destructive to the mind and
hurtful of the heart.
-- Henchick in Dark Tower, Song of Susannah by Stephen King

Lisa
Wed Oct 24 08:51:47 PDT 2007

Thanks for your feedback.

I do error checking, and I am very aware of the m_lpszClassName pointer
being risky, and I make sure I use it properly.
I am trying to bugfix some existing code from someone else. It's too much to
rewrite at this point. I am interested in general pointers, however, that's
not currently the issue at hand. I want to know why certain constructions
don't work properly, not why it's bad practice in general.

As for the LoadIcon one.. no, it's not the reason for the problem.. but it
is strange to me why following occurs, which may be related:


CSomeClass
{
CSomeClass(HINSTANCE hInstance) { m_hInstance = hInstance; };
virtual ~CSomeClass(){};
HINSTANCE m_hInstance;
HICON LoadIcon(UINT nID) {
return (HICON)LoadImage(m_hInstance,
MAKEINTRESOURCE(IDI_APP_ICON),
IMAGE_ICON, 16, 16, LR_DEFAULTCOLOR);
}
};

// global var
HINSTANCE g_hInstance = NULL;

BOOL APIENTRY DllMain(HANDLE hModule, DWORD dwReason, LPVOID pReserved)
{
switch(dwReason)
{
case DLL_PROCESS_ATTACH:
g_hInstance = (HINSTANCE) hModule;
break;
}
return TRUE;
}

// exported func
void SomeExportedFunc()
{
CSomeClass* p = new CSomeClass(g_hInstance);

HICON hIcon;

// this FAILS
hIcon = p->LoadIcon(IDI_ICON);

// this SUCCEEDS
hIcon = (HICON)LoadImage(m_hInstance, MAKEINTRESOURCE(IDI_ICON),
IMAGE_ICON, 16, 16, LR_DEFAULTCOLOR);

if (hIcon) DestroyIcon(hIcon);
}

Any clues?

There does seem to be some issue with instance handles or something..
I just don't understand this.

Lisa

Lisa
Wed Oct 24 08:57:29 PDT 2007

> Should this be __stdcall?

No, MSFT documentation defines it without that..

> Maybe there is some stack corruption due to uncorrect calling conventions?

Yes, I am wondering about something like this..
The exported function is called by its Ordinal, so that succeeds, but
unclear what happens with rest of the code.
It may be CE specific issue.

Lisa


"Giovanni Dicanio" <giovanni.dicanio@invalid.it> wrote in message
news:O8h5ohkFIHA.936@TK2MSFTNGP06.phx.gbl...
>
> "Lisa Pearlson" <no@spam.plz> ha scritto nel messaggio
> news:uyyHzvjFIHA.1204@TK2MSFTNGP03.phx.gbl...
>
>> // sample class
>> class CMyWnd
>> {
>> public:
>> CMyWnd() :
>> m_hWnd(NULL), m_lpszClassName(NULL),
>> m_hInstance(NULL), m_hIcon(NULL) {};
>
>> LPTSTR m_lpszClassName;
>
> I don't like this data member here.
> I would prefer storing the class name into a buffer using deep-copy (e.g.
> TCHAR m_className[ 200 ];) or better use a robust string class, like
> CString), e.g.:
>
> CString m_className;
>
> And adjust here:
>
>> BOOL RegisterWndClass(LPCTSTR lpszClassName, HINSTANCE hInstance,
>> WNDPROC wndProc) {
>> m_lpszClassName = lpszClassName;
>
> // Deep copy
> m_className = lpszClassName;
>
>> void UnregisterWndClass() {
>> UnregisterClass(m_lpszClassName, m_hInstance);
>> }
>
> I'm not sure if the class should be unregistered here...
> I would *not* unregister the window class.
>
> http://blogs.msdn.com/oldnewthing/archive/2007/02/12/1661754.aspx
>
>
>> // global vars. Do they need to be declared static???
>> static const g_szClassName[] = _T("MyClass");
>> static HINSTANCE g_hInstance = NULL;
>> static CMyWnd* pWnd = NULL;
>
>
>> LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM
>> lParam)
>> {
>> return pWnd->OnMessage(uMsg, wParam, lParam);
>
> I would check if pWnd is not null here, e.g. using an assert:
>
> _ASSERTE( pWnd != NULL );
>
>
>> case DLL_PROCESS_DETACH :
>> if (pWnd) {
>> pWnd->UnregisterWndClass();
>> delete pWnd;
>
> Try not unregistering here, and after delete pWnd, set also pWnd = NULL
> (just to avoid the use of a uninitialized pointer).
>
>> // .def file has EXPORTS Initialize @ 240 NONAME
>> HWND Initialize(HWND hWndParent)
>
> Should this be __stdcall?
> Maybe there is some stack corruption due to uncorrect calling conventions?
>
> However, I would not use this kind of "mixed C - C++" code; I would go for
> a more robust C++ code.
> I would use ATL/WTL (I don't know embedded development, maybe you have
> memory limits so you can't use MFC? But in that case I think you could use
> ATL, because ATL is a very *thin* object-oriented wrapper to Win32 APIs -
> and you can have the benefit of robust object-oriented code).
>
> Giovanni
>
>

Lisa
Wed Oct 24 09:16:04 PDT 2007

> I would *not* unregister the window class.

I read the web page you showed, why not to unregister.

I am wondering though.. window classes are system wide, right?
what happens if I reboot my OS? does it retain the class info?
Risk does increase that you will have class conflicts, when 2 different
applications register same name.

And what happens if another version of the application registers the same
class, but has renamed the wndproc or something?
Basically, the window class profile points to a wndproc that no longer
exists in memory.. I don't really understand this.

I do know that it's recommended (by MSFT docs) to always Unregister your
class before you register.. even though unregistering will fail first time,
if class hasn't been registered before, but it will ensure the call to
RegisterClass will succeed, even if it were previously registered.
What do you think of this?

Lisa

Igor
Wed Oct 24 09:27:39 PDT 2007

Lisa Pearlson <no@spam.plz> wrote:
> I am wondering though.. window classes are system wide, right?

No, they are process-wide. That's why you have to call
InitCommonControlsEx in every application that wants to use common
controls like list view.

> what happens if I reboot my OS? does it retain the class info?

Of course not.

> Risk does increase that you will have class conflicts, when 2
> different applications register same name.

Again, class registration is per process.
--
With best wishes,
Igor Tandetnik

With sufficient thrust, pigs fly just fine. However, this is not
necessarily a good idea. It is hard to be sure where they are going to
land, and it could be dangerous sitting under them as they fly
overhead. -- RFC 1925

Alex
Wed Oct 24 09:30:55 PDT 2007

"Lisa Pearlson" wrote:
>> I would *not* unregister the window class.
>
> I read the web page you showed, why not to unregister.
>
> I am wondering though.. window classes are system wide,
> right?
> what happens if I reboot my OS? does it retain the class
> info?
> Risk does increase that you will have class conflicts,
> when 2 different applications register same name.

No, windows classes are registered for each process. System
classes are registered implicitly for your process on first
call to User or GDI function.

> And what happens if another version of the application
> registers the same class, but has renamed the wndproc or
> something?

Each process gets its own copy of window class (including
system classes).

Alex

Igor
Wed Oct 24 09:37:49 PDT 2007

Lisa Pearlson <no@spam.plz> wrote:
> As for the LoadIcon one.. no, it's not the reason for the problem..
> but it is strange to me why following occurs, which may be related:
>
>
> CSomeClass
> {
> CSomeClass(HINSTANCE hInstance) { m_hInstance = hInstance; };
> virtual ~CSomeClass(){};
> HINSTANCE m_hInstance;
> HICON LoadIcon(UINT nID) {
> return (HICON)LoadImage(m_hInstance,
> MAKEINTRESOURCE(IDI_APP_ICON),
> IMAGE_ICON, 16, 16, LR_DEFAULTCOLOR);

Is it your intention that LoadIcon ignore its parameter and use a
hard-coded ID of IDI_APP_ICON instead?

> // exported func
> void SomeExportedFunc()
> {
> CSomeClass* p = new CSomeClass(g_hInstance);
>
> HICON hIcon;
>
> // this FAILS
> hIcon = p->LoadIcon(IDI_ICON);
>
> // this SUCCEEDS
> hIcon = (HICON)LoadImage(m_hInstance,
> MAKEINTRESOURCE(IDI_ICON), IMAGE_ICON,
> 16, 16, LR_DEFAULTCOLOR);

What's m_hInstance? As far as I can tell, it's an undeclared identifier
at this point.

As to why p->LoadIcon fails and LoadImage succeeds, note that the two
calls pass different IDs to LoadImage. Could it be that IDI_APP_ICON is
not a valid ID, while IDI_ICON is?
--
With best wishes,
Igor Tandetnik

With sufficient thrust, pigs fly just fine. However, this is not
necessarily a good idea. It is hard to be sure where they are going to
land, and it could be dangerous sitting under them as they fly
overhead. -- RFC 1925

Lisa
Wed Oct 24 10:41:18 PDT 2007

IDI_APP_ICON should've been nID
It's correct in my code, just mistyped it here.

So assume nID, note that m_hInstance is set in constructor, and is one
passed to DllMain.
Still, the global LoadImage succeeds while class version fails.

Lisa


"Igor Tandetnik" <itandetnik@mvps.org> wrote in message
news:%23ZAO4wlFIHA.3672@TK2MSFTNGP02.phx.gbl...
> Lisa Pearlson <no@spam.plz> wrote:
>> As for the LoadIcon one.. no, it's not the reason for the problem..
>> but it is strange to me why following occurs, which may be related:
>>
>>
>> CSomeClass
>> {
>> CSomeClass(HINSTANCE hInstance) { m_hInstance = hInstance; };
>> virtual ~CSomeClass(){};
>> HINSTANCE m_hInstance;
>> HICON LoadIcon(UINT nID) {
>> return (HICON)LoadImage(m_hInstance,
>> MAKEINTRESOURCE(IDI_APP_ICON),
>> IMAGE_ICON, 16, 16, LR_DEFAULTCOLOR);
>
> Is it your intention that LoadIcon ignore its parameter and use a
> hard-coded ID of IDI_APP_ICON instead?
>
>> // exported func
>> void SomeExportedFunc()
>> {
>> CSomeClass* p = new CSomeClass(g_hInstance);
>>
>> HICON hIcon;
>>
>> // this FAILS
>> hIcon = p->LoadIcon(IDI_ICON);
>>
>> // this SUCCEEDS
>> hIcon = (HICON)LoadImage(m_hInstance,
>> MAKEINTRESOURCE(IDI_ICON), IMAGE_ICON,
>> 16, 16, LR_DEFAULTCOLOR);
>
> What's m_hInstance? As far as I can tell, it's an undeclared identifier at
> this point.
>
> As to why p->LoadIcon fails and LoadImage succeeds, note that the two
> calls pass different IDs to LoadImage. Could it be that IDI_APP_ICON is
> not a valid ID, while IDI_ICON is?
> --
> With best wishes,
> Igor Tandetnik
>
> With sufficient thrust, pigs fly just fine. However, this is not
> necessarily a good idea. It is hard to be sure where they are going to
> land, and it could be dangerous sitting under them as they fly
> overhead. -- RFC 1925
>
>

Ben
Wed Oct 24 11:00:36 PDT 2007


"Igor Tandetnik" <itandetnik@mvps.org> wrote in message
news:%23fUsMrlFIHA.2004@TK2MSFTNGP06.phx.gbl...
> Lisa Pearlson <no@spam.plz> wrote:
>> I am wondering though.. window classes are system wide, right?
>
> No, they are process-wide. That's why you have to call
> InitCommonControlsEx in every application that wants to use common
> controls like list view.

I thought it might be different on CE, so I checked...
http://msdn2.microsoft.com/en-us/library/ms913089.aspx

This is probably the most relevant line: "All window classes that an
application registers are unregistered when it terminates."

>
>> what happens if I reboot my OS? does it retain the class info?
>
> Of course not.
>
>> Risk does increase that you will have class conflicts, when 2
>> different applications register same name.
>
> Again, class registration is per process.
> --
> With best wishes,
> Igor Tandetnik
>
> With sufficient thrust, pigs fly just fine. However, this is not
> necessarily a good idea. It is hard to be sure where they are going to
> land, and it could be dangerous sitting under them as they fly
> overhead. -- RFC 1925
>
>

Lisa
Wed Oct 24 11:33:56 PDT 2007

I'm using the SetWindowLong and GetWindowLong like so:



class MyWnd
{
BOOL Create(..)
{
m_hWnd = CreateWindow( .... );
SetWindowLong(hWnd, GWL_USERDATA, (LONG) hWnd);
}

virtual OnCreate(LPCREATESTRUCT lpcs)
{
return 0;
}

LRESULT OnMessage(MSG msg, WPARAM w, LPARAM l)
{
switch (msg)
{
case WM_CREATE:
return OnCreate( (LPCREATESTRUCT) lParam);
}
}
}

in WndProc I do:

LRESULT WndProc(HWND hWnd, MSG msg, WPARAM w, LPARAM l)
{
MyWnd* pWnd = reinterpret_cast<MyWnd*>( GetWindowLong(hWnd,
GWL_USERDATA);
return pWnd->OnMessage(msg, w, l);
}

The problem with this is construction is that WM_CREATE will never get
called properly because SetWindowLong occurs AFTER window has been created,
and I think WM_CREATE is sent just before..

So not sure how to solve this problem..

Lisa

Ben
Wed Oct 24 14:30:51 PDT 2007


"Lisa Pearlson" <no@spam.plz> wrote in message
news:OrgRWwmFIHA.4196@TK2MSFTNGP04.phx.gbl...
> I'm using the SetWindowLong and GetWindowLong like so:
>
>
>
> class MyWnd
> {
> BOOL Create(..)
> {
> m_hWnd = CreateWindow( .... );
> SetWindowLong(hWnd, GWL_USERDATA, (LONG) hWnd);
> }
>
> virtual OnCreate(LPCREATESTRUCT lpcs)
> {
> return 0;
> }
>
> LRESULT OnMessage(MSG msg, WPARAM w, LPARAM l)
> {
> switch (msg)
> {
> case WM_CREATE:
> return OnCreate( (LPCREATESTRUCT) lParam);
> }
> }
> }
>
> in WndProc I do:
>
> LRESULT WndProc(HWND hWnd, MSG msg, WPARAM w, LPARAM l)
> {
> MyWnd* pWnd = reinterpret_cast<MyWnd*>( GetWindowLong(hWnd,
> GWL_USERDATA);
> return pWnd->OnMessage(msg, w, l);
> }
>
> The problem with this is construction is that WM_CREATE will never get
> called properly because SetWindowLong occurs AFTER window has been
> created, and I think WM_CREATE is sent just before..
>
> So not sure how to solve this problem..

The static WndProc needs to handle WM_CREATE itself.

LRESULT WndProc(HWND hWnd, MSG msg, WPARAM wParam, LPARAM lParam)
{
MyWnd* pWnd;
if (msg == WM_CREATE) {
const CREATESTRUCT const* pCS = (const CREATESTRUCT const*)lParam;
pWnd = reinterpret_cast<MyWnd*>(pCS->lpCreateParams);
SetWindowLongPtr(hWnd, GWL_USERDATA, pWnd);
}
else
pWnd = reinterpret_cast<MyWnd*>( GetWindowLongPtr(hWnd,
GWL