[PATCH] winex11.drv: Stop assuming that GenericEvent has a window.

[Rémi Bernon]

It doesn't, and we then lookup HWND from an invalid Window value.

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/winex11.drv/event.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c
index d21d2a7fb1b..695e71b690f 100644
--- a/dlls/winex11.drv/event.c
+++ b/dlls/winex11.drv/event.c
@@ -390,8 +390,8 @@ static inline BOOL call_event_handler( Display *display, XEvent *event )
         return FALSE;  /* no handler, ignore it */
     }
 
-    if (XFindContext( display, event->xany.window, winContext, (char **)&hwnd ) != 0)
-        hwnd = 0;  /* not for a registered window */
+    if (event->type == GenericEvent || XFindContext( display, event->xany.window, winContext, (char **)&hwnd ))
+        hwnd = NULL;  /* not for a registered window */
     if (!hwnd && event->xany.window == root_window) hwnd = GetDesktopWindow();
 
     TRACE( "%lu %s for hwnd/window %p/%lx\n",
-- 
2.30.0

[Dmitry Timoshkov]

Rémi Bernon <rbernon(a)codeweavers.com> wrote:
> -    if (XFindContext( display, event->xany.window, winContext, (char **)&hwnd ) != 0)
> -        hwnd = 0;  /* not for a registered window */
> +    if (event->type == GenericEvent || XFindContext( display, event->xany.window, winContext, (char **)&hwnd ))
> +        hwnd = NULL;  /* not for a registered window */

Changing 0 to NULL looks gratuitous.

-- 
Dmitry.

[Rémi Bernon]

On 2/15/21 11:25 AM, Dmitry Timoshkov wrote:
> Rémi Bernon <rbernon(a)codeweavers.com> wrote:
> > -    if (XFindContext( display, event->xany.window, winContext, (char **)&hwnd ) != 0)
> > -        hwnd = 0;  /* not for a registered window */
> > +    if (event->type == GenericEvent || XFindContext( display, event->xany.window, winContext, (char **)&hwnd ))
> > +        hwnd = NULL;  /* not for a registered window */
>
> Changing 0 to NULL looks gratuitous.

Sure, and the != 0 removal too. I think HWND are set with NULL 
elsewhere. I took the opportunity of the change to make things a bit 
more consistent.
-- 
Rémi Bernon <rbernon(a)codeweavers.com>

[Dmitry Timoshkov]

Rémi Bernon <rbernon(a)codeweavers.com> wrote:
> On 2/15/21 11:25 AM, Dmitry Timoshkov wrote:
> > Rémi Bernon <rbernon(a)codeweavers.com> wrote:
> > > -    if (XFindContext( display, event->xany.window, winContext, (char **)&hwnd ) != 0)
> > > -        hwnd = 0;  /* not for a registered window */
> > > +    if (event->type == GenericEvent || XFindContext( display, event->xany.window, winContext, (char **)&hwnd ))
> > > +        hwnd = NULL;  /* not for a registered window */
> >
> > Changing 0 to NULL looks gratuitous.
>
> Sure, and the != 0 removal too. I think HWND are set with NULL 
> elsewhere. I took the opportunity of the change to make things a bit 
> more consistent.

Window handles are not pointers, please avoid gratuitous changes in the diffs.

-- 
Dmitry.

[Jefferson Carpenter]

On 2/15/2021 11:26 AM, Dmitry Timoshkov wrote:
> Rémi Bernon <rbernon(a)codeweavers.com> wrote:
> > On 2/15/21 11:25 AM, Dmitry Timoshkov wrote:
> > > Rémi Bernon <rbernon(a)codeweavers.com> wrote:
> > > > -    if (XFindContext( display, event->xany.window, winContext, (char **)&hwnd ) != 0)
> > > > -        hwnd = 0;  /* not for a registered window */
> > > > +    if (event->type == GenericEvent || XFindContext( display, event->xany.window, winContext, (char **)&hwnd ))
> > > > +        hwnd = NULL;  /* not for a registered window */
> > >
> > > Changing 0 to NULL looks gratuitous.
> >
> > Sure, and the != 0 removal too. I think HWND are set with NULL
> > elsewhere. I took the opportunity of the change to make things a bit
> > more consistent.
>
> Window handles are not pointers, please avoid gratuitous changes in the diffs.

Question is what the proper null value is for an HWND: 0 or NULL. 
Semantically, I think NULL is the proper choice - an MSDN example 
compares an HWND against NULL [1].  Also in Wine the underlying type of 
HWND is a pointer type [2][3] and the NULL macro is defined to compare 
equal to a null pointer, so it seems NULL is the right choice.

[1] 
https://docs.microsoft.com/en-us/windows/win32/learnwin32/creating-a-window
[2] https://github.com/wine-mirror/wine/blob/wine-6.2/include/windef.h#L333
[3] https://github.com/wine-mirror/wine/blob/wine-6.2/include/winnt.h#L583

[Francois Gouget]

On Mon, 15 Feb 2021, Dmitry Timoshkov wrote:
[...]
> Window handles are not pointers, please avoid gratuitous changes in the diffs.

include/windef.h:#ifndef NO_STRICT
include/windef.h:# ifndef STRICT
include/windef.h:#  define STRICT
include/windef.h:# endif /* STRICT */

So Wine compiles with STRICT.

include/winnt.h:#ifdef STRICT
include/winnt.h:#define DECLARE_HANDLE(a) typedef struct a##__ { int 
unused; } *a
include/winnt.h:#else /*STRICT*/
include/winnt.h:#define DECLARE_HANDLE(a) typedef HANDLE a
include/winnt.h:#endif /*STRICT*/

And DECLARE_HANDLE() creates pointer types.

So from the compiler point of view they are pointers.
> From an implementation point of view... well, that's irrelevant: 
> handle-using code is not supposed to care about how handles are 
> implemented.

-- 
Francois Gouget <fgouget(a)free.fr>              http://fgouget.free.fr/
                            $live{free} || die "";