Re: Kusanagi Kouichi : winex11: Destroy the handle to the composition string after it is cancelled.

[Aric Stewart]

Comming in a bit late on this one, (missed the patch submission)

I do not think this is correct as far as i know hCompStr should never be 
NULL. Much of our code depends on that not being NULL.

I would recommend using ImeCreateBlankCompStr instead. (something like 
below)

diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c
index f50549f..8fa6f6f 100644
--- a/dlls/winex11.drv/ime.c
+++ b/dlls/winex11.drv/ime.c
@@ -778,7 +778,7 @@ BOOL WINAPI NotifyIME(HIMC hIMC, DWORD dwAction, 
DWORD dwInd
                      if (lpIMC->hCompStr)
                      {
                          ImmDestroyIMCC(lpIMC->hCompStr);
-                        lpIMC->hCompStr = NULL;
+                        lpIMC->hCompStr = ImeCreateBlankCompStr();
                      }

                      myPrivate = 
(LPIMEPRIVATE)ImmLockIMCC(lpIMC->hPrivate);

-aric

Alexandre Julliard wrote:
> Module: wine
> Branch: master
> Commit: 4f2330ac0c951ec5fb05e2fb152c59723c3af336
> URL:    http://source.winehq.org/git/wine.git/?a=commit;h=4f2330ac0c951ec5fb05e2fb15...
>
> Author: Kusanagi Kouichi <slash(a)ma.neweb.ne.jp>
> Date:   Fri Apr 25 12:08:24 2008 +0900
>
> winex11: Destroy the handle to the composition string after it is cancelled.
>
> ---
>
> dlls/winex11.drv/ime.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c
> index 6614213..f50549f 100644
> --- a/dlls/winex11.drv/ime.c
> +++ b/dlls/winex11.drv/ime.c
> @@ -775,6 +775,12 @@ BOOL WINAPI NotifyIME(HIMC hIMC, DWORD dwAction, DWORD dwIndex, DWORD dwValue)
>
> X11DRV_ForceXIMReset(lpIMC->hWnd);
>
> +                    if (lpIMC->hCompStr)
> +                    {
> +                        ImmDestroyIMCC(lpIMC->hCompStr);
> +                        lpIMC->hCompStr = NULL;
> +                    }
> +
>                      myPrivate = (LPIMEPRIVATE)ImmLockIMCC(lpIMC->hPrivate);
>                      if (myPrivate->bInComposition)
>                      {

Kusanagi Kouichi : winex11: Destroy the handle to the composition string after it is cancelled.

[Kusanagi Kouichi]

On Fri, Apr 25, 2008 at 09:36:36AM -0500, Aric Stewart wrote:
> Comming in a bit late on this one, (missed the patch submission)
>
> I do not think this is correct as far as i know hCompStr should never be 
> NULL. Much of our code depends on that not being NULL.
>
> I would recommend using ImeCreateBlankCompStr instead. (something like 
> below)
>
> diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c
> index f50549f..8fa6f6f 100644
> --- a/dlls/winex11.drv/ime.c
> +++ b/dlls/winex11.drv/ime.c
> @@ -778,7 +778,7 @@ BOOL WINAPI NotifyIME(HIMC hIMC, DWORD dwAction, 
> DWORD dwInd
>                       if (lpIMC->hCompStr)
>                       {
>                           ImmDestroyIMCC(lpIMC->hCompStr);
> -                        lpIMC->hCompStr = NULL;
> +                        lpIMC->hCompStr = ImeCreateBlankCompStr();
>                       }
>
> myPrivate = 
> (LPIMEPRIVATE)ImmLockIMCC(lpIMC->hPrivate);
>
> -aric

Then, are these also wrong? updateCompStr returns NULL if NULL or 0 is given.
   748	                        newCompStr = updateCompStr(lpIMC->hCompStr, NULL, 0);
   749	                        ImmDestroyIMCC(lpIMC->hCompStr);
   750	                        lpIMC->hCompStr = newCompStr;

   897	            newCompStr = updateCompStr(lpIMC->hCompStr, NULL, 0);
   898	            ImmDestroyIMCC(lpIMC->hCompStr);
   899	            lpIMC->hCompStr = newCompStr;

Kusanagi Kouichi : winex11: Destroy the handle to the composition string after it is cancelled.

[Aric Stewart]

No, from updateCompStr

     if (old == NULL && compstr == NULL && len == 0)
         return NULL;

Otherwise it returns a compstr. So it only should return NULL if all the 
incoming parameters are NULL, which i do not believe should ever happen 
after i went through and made sure that the hCompStr was never NULL.

-aric

Kusanagi Kouichi wrote:
> On Fri, Apr 25, 2008 at 09:36:36AM -0500, Aric Stewart wrote:
> > Comming in a bit late on this one, (missed the patch submission)
> >
> > I do not think this is correct as far as i know hCompStr should never be 
> > NULL. Much of our code depends on that not being NULL.
> >
> > I would recommend using ImeCreateBlankCompStr instead. (something like 
> > below)
> >
> > diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c
> > index f50549f..8fa6f6f 100644
> > --- a/dlls/winex11.drv/ime.c
> > +++ b/dlls/winex11.drv/ime.c
> > @@ -778,7 +778,7 @@ BOOL WINAPI NotifyIME(HIMC hIMC, DWORD dwAction, 
> > DWORD dwInd
> >                       if (lpIMC->hCompStr)
> >                       {
> >                           ImmDestroyIMCC(lpIMC->hCompStr);
> > -                        lpIMC->hCompStr = NULL;
> > +                        lpIMC->hCompStr = ImeCreateBlankCompStr();
> >                       }
> >
> > myPrivate = 
> > (LPIMEPRIVATE)ImmLockIMCC(lpIMC->hPrivate);
> >
> > -aric
>
> Then, are these also wrong? updateCompStr returns NULL if NULL or 0 is given.
>    748	                        newCompStr = updateCompStr(lpIMC->hCompStr, NULL, 0);
>    749	                        ImmDestroyIMCC(lpIMC->hCompStr);
>    750	                        lpIMC->hCompStr = newCompStr;
>
> 897	            newCompStr = updateCompStr(lpIMC->hCompStr, NULL, 0);
>    898	            ImmDestroyIMCC(lpIMC->hCompStr);
>    899	            lpIMC->hCompStr = newCompStr;

Kusanagi Kouichi : winex11: Destroy the handle to the composition string after it is cancelled.

[Kusanagi Kouichi]

On Fri, Apr 25, 2008 at 02:44:00PM -0500, Aric Stewart wrote:
> No, from updateCompStr
>
> if (old == NULL && compstr == NULL && len == 0)
>          return NULL;
>
> Otherwise it returns a compstr. So it only should return NULL if all the 
> incoming parameters are NULL, which i do not believe should ever happen 
> after i went through and made sure that the hCompStr was never NULL.
>
> -aric

Oh, I mistook && for ||. All right, I'll send a patch.