Hello,
while trying to convert HHOOK to a void* i found this strangeness: CallNextHookEx16 expects its first parameter to be a real HHOOK, that means it checks the HIWORD of the HHOOK to see it it contains 'HK' if (HIWORD(hhook) != HOOK_MAGIC) return 0; /* Not a new format hook */
In wine there are only 2 functions that call CallNextHookEx16, one is DefHookProc16, the other is ShellHookProc16. The first one calls CallNextHookEx16 with first parameter queue->hCurHook (an HANDLE16), the second one with WH_SHELL (the number 10). This parameters gets implicitly transformed to a HHOOK and will have their HIWORD set to 0, thus CallNextHookEx16 will always fail and return 0.
I don't know what the right fix would be, I'm seeing two possibilities: - remove in CallNextHookEx16 the check if (HIWORD(hhook) != HOOK_MAGIC) return 0; - or transform in DefHookProc16 and ShellHookProc16 the first parameter passed to CallNextHookEx16 to a real HHOOK with the HIWORD set to 'HC'
Comments? bye michael
Michael Stefaniuc mstefani@redhat.com writes:
I don't know what the right fix would be, I'm seeing two possibilities:
- remove in CallNextHookEx16 the check if (HIWORD(hhook) != HOOK_MAGIC) return 0;
- or transform in DefHookProc16 and ShellHookProc16 the first parameter passed to CallNextHookEx16 to a real HHOOK with the HIWORD set to 'HC'
The second is right, we should always pass a real HHOOK. In fact a hook handle is 32 bit even in Win16, so it's never correct to assign it to a HANDLE16; we do this at a few other places too, that should probably be fixed.
On Tue, Jul 23, 2002 at 12:01:25PM -0700, Alexandre Julliard wrote:
Michael Stefaniuc mstefani@redhat.com writes:
I don't know what the right fix would be, I'm seeing two possibilities:
- remove in CallNextHookEx16 the check if (HIWORD(hhook) != HOOK_MAGIC) return 0;
- or transform in DefHookProc16 and ShellHookProc16 the first parameter passed to CallNextHookEx16 to a real HHOOK with the HIWORD set to 'HC'
The second is right, we should always pass a real HHOOK. In fact a hook handle is 32 bit even in Win16, so it's never correct to assign it to a HANDLE16; we do this at a few other places too, that should probably be fixed.
I've changed DECLARE_OLD_HANDLE(HHOOK) to DECLARE_HANDLE(HOOK) and I'm compiling wine with -DSTRICT and the two above functions were the only ones that did an implicit conversion from a HANDLE16 to a HHOOK. There is only one cast to HHANDLE but that should be ok: return (HHOOK)( handle? MAKELONG( handle, HOOK_MAGIC ) : 0 );
It seems that we don't have an implicit transformation of a HHOOK to a HANDLE16. Most of the internal HOOK_* functions are using HANDLE16's and the few functions that accepts as parameter a HHOOK are checking for the presence of the HOOK_MAGIC and are doing the conversion to a HANDLE16 with LOWORD(hhook). I've also looked over the casts to a HANDLE16 and it seems that no HHOOK is transformed this way.
The attached patch fixes the bug and also converts HHOOK to a void*
License: LGPL, X11 Changelog: Michael Stefaniuc mstefani@redhat.com - fix the calls to CallNextHookEx16() - convert HHOOK to a void*
bye michael
Michael Stefaniuc mstefani@redhat.de writes:
It seems that we don't have an implicit transformation of a HHOOK to a HANDLE16. Most of the internal HOOK_* functions are using HANDLE16's and the few functions that accepts as parameter a HHOOK are checking for the presence of the HOOK_MAGIC and are doing the conversion to a HANDLE16 with LOWORD(hhook).
Well yes, the hook functions are mostly using HANDLE16 internally, but it would be much better to change them to use HHOOK. There isn't much point in making HHOOK work with -DSTRICT if we don't use it anywhere.
- return CallNextHookEx16( WH_SHELL, code, wParam, lParam ); + return CallNextHookEx16( (HHOOK)MAKELONG(WH_SHELL, HOOK_MAGIC), code, + wParam, lParam );
WH_SHELL is not a valid hook handle, this code is broken (of course it was broken before too).
On Tue, Jul 23, 2002 at 03:18:04PM -0700, Alexandre Julliard wrote:
Michael Stefaniuc mstefani@redhat.de writes:
It seems that we don't have an implicit transformation of a HHOOK to a HANDLE16. Most of the internal HOOK_* functions are using HANDLE16's and the few functions that accepts as parameter a HHOOK are checking for the presence of the HOOK_MAGIC and are doing the conversion to a HANDLE16 with LOWORD(hhook).
Well yes, the hook functions are mostly using HANDLE16 internally, but it would be much better to change them to use HHOOK. There isn't much point in making HHOOK work with -DSTRICT if we don't use it anywhere.
Hmm ... looking at windows/hook.c i guess that we have to keep using with HOOKDATA, MESSAGEQUEUE, USER_HEAP_LIN_ADDR, etc. and this ones are using HANDLE16. HOOK_systemHooks could be changed to HHOOK, but this would introduce a lot of conversions. I've changed all internal HOOK_* functions to pass only HHOOK's to each other. I've kept the use of HANDLE16 in functions that make extensive use of HOOKDATA and MESSAGEQUEUE but changed the variables name from hook to handle to not produce confusion.
- return CallNextHookEx16( WH_SHELL, code, wParam, lParam );
- return CallNextHookEx16( (HHOOK)MAKELONG(WH_SHELL, HOOK_MAGIC), code,
wParam, lParam );WH_SHELL is not a valid hook handle, this code is broken (of course it was broken before too).
ShellHookProc is only a win < win95 function and I couldn't find any documentation for it (searching on msdn revealed nothing) so i've made a separat patch for it, but it's a pure guesstimation.
License: LGPL, X11 Changelog: Michael Stefaniuc mstefani@redhat.com - converted HHOOK to a void* - changed the internal HOOK_* functions to pass only HHOOK's between them - fixed wrong HHOOK <-> HANDLE16 conversions
bye michael
-
Alexandre Julliard -
Michael Stefaniuc -
Michael Stefaniuc