Re: CallNextHookEx16 strageness

2002-07-27 Thread Michael Stefaniuc

On Tue, Jul 23, 2002 at 03:18:04PM -0700, Alexandre Julliard wrote:
> Michael Stefaniuc <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
- converted HHOOK to a void*
- changed the internal HOOK_* functions to pass only HHOOK's between
  them
- fixed wrong HHOOK <-> HANDLE16 conversions

bye
michael
-- 
Michael Stefaniuc   Tel.: +49-711-96437-199
System Administration   Fax.: +49-711-96437-111
Red Hat GmbHEmail: [EMAIL PROTECTED]
Hauptstaetterstr. 58http://www.redhat.de/
D-70178 Stuttgart


Index: include/windef.h
===
RCS file: /home/wine/wine/include/windef.h,v
retrieving revision 1.65
diff -u -r1.65 windef.h
--- include/windef.h19 Jul 2002 00:28:13 -  1.65
+++ include/windef.h27 Jul 2002 17:13:00 -
@@ -79,7 +79,7 @@
 DECLARE_HANDLE(HDESK);
 DECLARE_OLD_HANDLE(HENHMETAFILE);
 DECLARE_OLD_HANDLE(HFONT);
-DECLARE_OLD_HANDLE(HHOOK);
+DECLARE_HANDLE(HHOOK);
 DECLARE_OLD_HANDLE(HICON);
 DECLARE_OLD_HANDLE(HINSTANCE);
 DECLARE_OLD_HANDLE(HKEY);
Index: windows/hook.c
===
RCS file: /home/wine/wine/windows/hook.c,v
retrieving revision 1.35
diff -u -r1.35 hook.c
--- windows/hook.c  10 Jul 2002 23:20:49 -  1.35
+++ windows/hook.c  27 Jul 2002 17:13:02 -
@@ -61,6 +61,8 @@
 #include "poppack.h"
 
 #define HOOK_MAGIC  ((int)'H' | (int)'K' << 8)  /* 'HK' */
+#define HHOOK_32(h) ((HHOOK)(h ? MAKELONG(h, HOOK_MAGIC) : 0))
+#define HHOOK_16(h) ((HANDLE16)((HIWORD(h) == HOOK_MAGIC) ? LOWORD(h) : 0))
 
   /* This should probably reside in USER heap */
 static HANDLE16 HOOK_systemHooks[WH_NB_HOOKS] = { 0, };
@@ -590,16 +592,16 @@
  *
  * Get the next hook of a given hook.
  */
-static HANDLE16 HOOK_GetNextHook( HANDLE16 hook )
+static HHOOK HOOK_GetNextHook( HHOOK hook )
 {
-HOOKDATA *data = (HOOKDATA *)USER_HEAP_LIN_ADDR( hook );
+HOOKDATA *data = (HOOKDATA *)USER_HEAP_LIN_ADDR(HHOOK_16(hook));
 
 if (!data || !hook) return 0;
-if (data->next) return data->next;
+if (data->next) return HHOOK_32(data->next);
 if (!data->ownerQueue) return 0;  /* Already system hook */
 
 /* Now start enumerating the system hooks */
-return HOOK_systemHooks[data->id - WH_MINHOOK];
+return HHOOK_32(HOOK_systemHooks[data->id - WH_MINHOOK]);
 }
 
 
@@ -608,15 +610,15 @@
  *
  * Get the first hook for a given type.
  */
-static HANDLE16 HOOK_GetHook( INT16 id )
+static HHOOK HOOK_GetHook( INT16 id )
 {
 MESSAGEQUEUE *queue;
-HANDLE16 hook = 0;
+HANDLE16 handle = 0;
 
 if ((queue = QUEUE_Current()) != NULL)
-hook = queue->hooks[id - WH_MINHOOK];
-if (!hook) hook = HOOK_systemHooks[id - WH_MINHOOK];
-return hook;
+handle = queue->hooks[id - WH_MINHOOK];
+if (!handle) handle = HOOK_systemHooks[id - WH_MINHOOK];
+return HHOOK_32(handle);
 }
 
 
@@ -677,7 +679,7 @@
 TRACE("Setting hook %d: ret=%04x [next=%04x]\n",
   id, handle, data->next );
 
-return (HHOOK)( handle? MAKELONG( handle, HOOK_MAGIC ) : 0 );
+return HHOOK_32(handle);
 }
 
 
@@ -686,14 +688,14 @@
  *
  * Remove a hook from the list.
  */
-static BOOL HOOK_RemoveHook( HANDLE16 hook )
+static BOOL HOOK_RemoveHook( HHOOK hook )
 {
 HOOKDATA *data;
-HANDLE16 *prevHook;

Re: CallNextHookEx16 strageness

2002-07-23 Thread Alexandre Julliard

Michael Stefaniuc <[EMAIL PROTECTED]> 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).

-- 
Alexandre Julliard
[EMAIL PROTECTED]




Re: CallNextHookEx16 strageness

2002-07-23 Thread Alexandre Julliard

Michael Stefaniuc <[EMAIL PROTECTED]> 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.

-- 
Alexandre Julliard
[EMAIL PROTECTED]




CallNextHookEx16 strageness

2002-07-22 Thread Michael Stefaniuc

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   Tel.: +49-711-96437-199
System Administration   Fax.: +49-711-96437-111
Red Hat GmbHEmail: [EMAIL PROTECTED]
Hauptstaetterstr. 58http://www.redhat.de/
D-70178 Stuttgart



msg10967/pgp0.pgp
Description: PGP signature