Re: FW: shell32.SHDefExtractIcon implementation take

[Uwe Bonnes]

> > > > > "Rolf" == Rolf Kalbermatter  writes:

+/****************************************************************************
+ * SHDefExtractIconW           [SHELL32.@]
+ */
+HRESULT SHDefExtractIconW(LPCWSTR pszIconFile, int iIndex, UINT uFlags,
+                          HICON* phiconLarge, HICON* phiconSmall, UINT nIconSize)
+{
+       UINT ret;
+       HICON hIcons[2];
              ^^^^^^^^

+       WARN("%s %d 0x%08x %p %p %d, semi-stub\n", debugstr_w(pszIconFile),\
iIndex, uFlags, phiconLarge, phiconSmall, nIconSize);
+
+       ret = PrivateExtractIconsW(pszIconFile, iIndex, nIconSize,
nIconSize, h\
Icons, NULL, 2, LR_DEFAULTCOLOR);
+       /* FIXME: deal with uFlags parameter which contains GIL_ flags */
+       if (ret == 0xFFFFFFFF)
+         return E_FAIL;
+       if (ret > 0) {
+         *phiconLarge = hIcons[0];
          ^^^^^^^^^^^^
Rolf,

this doesn't look right to me:

You allocate space on the function local stack, then hand out a pointer to
that memory and return from the function. No the function local stack is
undefined and the pointers are dangling. I think the heap should be used for
allocating the needed storage.

Bye

FW: shell32.SHDefExtractIcon implementation take

[Rolf Kalbermatter]

Mmmh, at first sight I thought damn how could this happen but then looking
at it I don't think there is a problem. I do not return the memory which was
allocated on the stack, but the HICONS allocated and returned by
PrivateExtractIconsW in the hIcons array! So this shouldn't be a problem
at all or are my brain cells garbled that much already?

Rolf Kalbermatter
> -----Original Message-----
> From: Uwe Bonnes [mailto:[e-mail]]
> Sent: Friday, December 06, 2002 9:53 PM
> To: [e-mail]
> Cc: [e-mail]
> Subject: Re: FW: shell32.SHDefExtractIcon implementation take
> > > > > > "Rolf" == Rolf Kalbermatter  writes:
>
> +/****************************************************************************
> -
> - SHDefExtractIconW           [SHELL32.@]
> - */
>
> +HRESULT SHDefExtractIconW(LPCWSTR pszIconFile, int iIndex, UINT uFlags,
> -
>                      HICON* phiconLarge, HICON* phiconSmall, UINT nIconSize)
>
> +{
> -
>   UINT ret;
> -
>   HICON hIcons[2];
>         ^^^^^^^^
>
> -
>   WARN("%s %d 0x%08x %p %p %d, semi-stub\n", debugstr_w(pszIconFile),\
>
> iIndex, uFlags, phiconLarge, phiconSmall, nIconSize);
> -
> -
>   ret = PrivateExtractIconsW(pszIconFile, iIndex, nIconSize,
>
> nIconSize, hIcons, NULL, 2, LR_DEFAULTCOLOR);
> -
>   /* FIXME: deal with uFlags parameter which contains GIL_ flags */
> -
>   if (ret == 0xFFFFFFFF)
> -
>     return E_FAIL;
> -
>   if (ret > 0) {
> -
>     *phiconLarge = hIcons[0];
>     ^^^^^^^^^^^^
>
> Rolf,
>
> this doesn't look right to me:
>
> You allocate space on the function local stack, then hand out a pointer to
> that memory and return from the function. No the function local stack is
> undefined and the pointers are dangling. I think the heap should be used for
> allocating the needed storage.
>
> Bye
>
> -- 
> Uwe Bonnes                [e-mail]
>
> Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
> --------- Tel. [phone] -------- Fax. [phone] ----------