[PATCH 2/2] shell32/iconcache: Break as soon as one shortcut failed to overlay

[Gabriel Ivăncescu]

On Thu, Nov 29, 2018 at 1:13 PM Huw Davies <huw at codeweavers.com> wrote:
>
> On Mon, Nov 26, 2018 at 12:22:04PM +0200, Gabriel Ivăncescu wrote:
> > There is no need to initialize and destroy shortcut icons that we haven't
> > even processed.
> >
> > Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> > ---
> >  dlls/shell32/iconcache.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/dlls/shell32/iconcache.c b/dlls/shell32/iconcache.c
> > index 44422ab..b9264bb 100644
> > --- a/dlls/shell32/iconcache.c
> > +++ b/dlls/shell32/iconcache.c
> > @@ -361,7 +361,6 @@ static BOOL get_imagelist_icon_size(int list, SIZE *size)
> >  static INT SIC_LoadIcon (const WCHAR *sourcefile, INT index, DWORD flags)
> >  {
> >      HICON hicons[ARRAY_SIZE(shell_imagelists)] = { 0 };
> > -    HICON hshortcuts[ARRAY_SIZE(hicons)] = { 0 };
> >      unsigned int i;
> >      SIZE size;
> >      INT ret = -1;
> > @@ -376,6 +375,7 @@ static INT SIC_LoadIcon (const WCHAR *sourcefile, INT index, DWORD flags)
> >
> >      if (flags & GIL_FORSHORTCUT)
> >      {
> > +        HICON hshortcuts[ARRAY_SIZE(hicons)];
> >          BOOL failed = FALSE;
> >
> >          for (i = 0; i < ARRAY_SIZE(hshortcuts); i++)
> > @@ -384,13 +384,15 @@ static INT SIC_LoadIcon (const WCHAR *sourcefile, INT index, DWORD flags)
> >              {
> >                  WARN("Failed to create shortcut overlaid icons.\n");
> >                  failed = TRUE;
> > +                break;
> >              }
> >          }
> >
> >          if (failed)
> >          {
> > -            for (i = 0; i < ARRAY_SIZE(hshortcuts); i++)
> > -                DestroyIcon(hshortcuts[i]);
> > +            unsigned int k;
> > +            for (k = 0; k < i; k++)
> > +                DestroyIcon(hshortcuts[k]);
> >              flags &= ~GIL_FORSHORTCUT;
> >          }
> >          else
>
> Rather than optimizing the failure case, we should focus on addressing
> the real problem.
>
> Huw.

By real problem, you mean the icon not being reconstructed from the
available ones? I planned to do that after, because the patch that
does it is a bit convoluted. Now I just sent the easy bits first.

I mean, I *totally* understand why that patch feels like a semi-hack.
At worst I was planning to get it into wine-staging, at least until we
find a better way to do it.

BTW, I asked Nikolay before but he didn't answer -- I'm not sure how
to make tests for that patch's scenario, because I don't know how to
reproduce the failure of only some icon sizes in a wine test (as in
the bug report via TurnItOn.exe). I mean, it's trivial to reproduce it
with TurnItOn.exe but I can't just ship that with the test obviously.