Re: A problem with comctl32

[Rolf Kalbermatter]

DanteAliegri  wrote:
> Hey, I've come across what appears to be a
> simple problem in comctl32.
> When running icq99b, wine was dying in imagelist.c while trying to 
> dereference a null pointer.
> Upon looking at the file,  there was code for returning FALSE if that 
> pointer was null, thus I felt it being null may be a valid choice.
> I made the attached change, and the problem was fixed.
> Comments?
>
> --- imagelist.c	23 Oct 2002 22:19:11 -0000	1.65
> +++ imagelist.c	2 Nov 2002 20:40:53 -0000
> @@ -1082,11 +1082,14 @@
>     HBITMAP hImageBmp, hOldImageBmp, hOldImageListBmp, hOldMaskListBmp,
> hBlendMaskBmp;
>     BOOL bIsTransparent, bBlend, bResult = FALSE;
>     const HIMAGELIST himl = pimldp->himl;

^^^^^^^^^^^^
According to the same lines pimldp could also be NULL so this might as
well cause a NULL pointer dereference and should be moved to after the
check for "if (!pimldp || !(himl = pimldp->himl)) return FALSE;

Of course the question remains why would you call the function at all
with a NULL pointer.
> - const INT lx = himl->cx * pimldp->i + pimldp->xBitmap;
> - const INT ly = pimldp->yBitmap;
> - static INT lx;
> - static INT ly;

Should this be really static? Can't this function be called reentrant?
> if (!pimldp || !himl) return FALSE;
> if ((pimldp->i < 0) || (pimldp->i >= himl->cCurImage)) return FALSE;
> -
> -
> lx = himl->cx * pimldp->i + pimldp->xBitmap;
> -
> ly = pimldp->yBitmap;

Rolf Kalbermatter

A problem with comctl32

[Dimitrie O. Paun]

On November 4, 2002 05:49 am, Rolf Kalbermatter wrote:
> > const HIMAGELIST himl = pimldp->himl;
>                                  ^^^^^^^^^^^^
>
> According to the same lines pimldp could also be NULL so this might as
> well cause a NULL pointer dereference and should be moved to after the
> check for "if (!pimldp || !(himl = pimldp->himl)) return FALSE;

Right, this needs to be moved below the if as well. The reasons they
are this way it's that we used to have internal helper functions that
were guaranteed these are not NULL. It's a copy & paste bug.
> Of course the question remains why would you call the function at all
> with a NULL pointer.

We can not control that, the function is directly callable by the app.