Hi Ivan.
--- Ivan Leo Puoti puoti@inwind.it wrote:
Please consider I'm no programming wiz and I'm learning as I go along.
No problem. I'll give a lengthy reply in the hope it can help other folks dealing with this sort of thing too.
Now, I don't see how else I could get MSGBOX_EnumProc() to remember the values of those that now are static variables, apart from using the heap for everything, that seems a bit inefficent as we know how much data we want to store, or global variables, do you think I should use global ones instead? If not suggestions are welcome.
Globals are also not thread-safe.
Generally for thread safety, I'd suggest the following: declare a structure that has all the data you want to modify in MSGBOX_EnumProc, something like: struct MsgBoxEnumProcParams { int pass; int counter1; int counter2; int heapcheck; HWND *handles; };
In MessageBoxIndirectW, create one of these and pass its address as the LPARAM value to EnumThreadWindows().
However, this ignores style issues. I have problems with names like "counter1" and "counter2"; they don't tell me what they mean. Also, I don't like the magic values 0, 1, 2, 3, 4 for the LPARAM value; at the least these should be symbolic constants.
Pass 3 doesn't modify the parameters at all, so it really should get its own callback function.
It looks like heapcheck's purpose is to see whether handles has in fact been allocated. You could instead make sure to set handles to NULL when it isn't allocated. Also, there's the possibility of a memory leak in pass 4. I'd suggest the caller should be responsible for memory allocation.
With these suggestions, I've come up with the following changes. I haven't checked whether this code works (or even compiles), it's just my attempt at rewriting what you're trying to do:
struct ThreadWindows { UINT numHandles; UINT handlesLen; HWND *handles; };
BOOL CALLBACK MSGBOX_CountDisabledThreadWindows(HWND hwnd, LPARAM lParam) { if (!lParam) return FALSE; if (!IsWindowEnabled(hwnd)) *(UINT *)lParam++; return TRUE; }
BOOL CALLBACK MSGBOX_EnumDisabledThreadWindows(HWND hwnd, LPARAM lParam) { if (!lParam) return FALSE; if (!IsWindowEnabled(hwnd)) { struct ThreadWindows *threadWindows = (struct ThreadWindows *)lParam;
if (threadWindows->numHandles < threadWindows->handlesLen) threadWindows->handles[threadWindows->numHandles++] = hwnd; } return TRUE; }
INT WINAPI MessageBoxIndirectW( LPMSGBOXPARAMSW msgbox ) { LPVOID tmplate; HRSRC hRes; int ret; UINT i; struct ThreadWindows threadWindows = { 0, 0, 0 }; static const WCHAR msg_box_res_nameW[] = { 'M','S','G','B','O','X',0 };
if (!(hRes = FindResourceExW(user32_module, (LPWSTR)RT_DIALOG, msg_box_res_nameW, msgbox->dwLanguageId))) return 0; if (!(tmplate = (LPVOID)LoadResource(user32_module, hRes))) return 0;
/*handle task modal message boxes*/ if ((msgbox->dwStyle & MB_TASKMODAL) && (msgbox->hwndOwner==NULL)) { EnumThreadWindows(GetCurrentThreadId(), MSGBOX_CountDisabledThreadWindows, (LPARAM)&threadWindows.numHandles); if (threadWindows.numHandles) { threadWindows.handles = (HWND *)HeapAlloc(GetProcessHeap(), 0, threadWindows.numHandles * sizeof(HWND)); if (threadWindows.handles) threadWindows.handlesLen = threadWindows.numHandles; threadWindows.numHandles = 0; } EnumThreadWindows(GetCurrentThreadId(), MSGBOX_EnumDisabledThreadWindows, (LPARAM)&threadWindows); for (i = 0; i < threadWindows.numHandles; i++) DIALOG_DisableOwner(threadWindows.handles[i]); }
ret=DialogBoxIndirectParamW(msgbox->hInstance, tmplate, msgbox->hwndOwner, MSGBOX_DlgProc, (LPARAM)msgbox);
if ((msgbox->dwStyle & MB_TASKMODAL) && (msgbox->hwndOwner==NULL)) { for (i = 0; i < threadWindows.numHandles; i++) DIALOG_EnableOwner(threadWindows.handles[i]); if (threadWindows.handles) HeapFree(GetProcessHeap(), 0, threadWindows.handles); }
return ret; }
The changes I've made:
I've renamed counter1 and counter2 to numHandles and handlesLen. handlesLen is the allocated number of handles, while numHandles is the number of handles actually stored. (Your code already dealt with the possibility of their being different: well done!)
I've also used handles as its own guard; if it's NULL, it's not allocated. Otherwise it is.
Like I suggested, I pass a pointer as an LPARAM to the two EnumThreadWindows callback functions.
It seems two calls to EnumThreadWindows were unnecessary, since we already knew the window handles, so I just iterate over the values in handles rather than calling EnumThreadWindows again. If I'm incorrect in my understanding, of course change it.
And, the caller is always responsible for memory allocation and freeing. I think this is safer than having the callback function do it.
Any reason you aren't using HeapAlloc instead?
As this heap is only used by MSGBOX_EnumProc(), is there any reason why I should use HeapAlloc?
It's sometimes easier to debug memory problems with the Windows heaps than with the libc ones.
--Juan
__________________________________ Do you Yahoo!? Check out the new Yahoo! Front Page. www.yahoo.com