Re: winemenubuilder: fix crash caused by invalid icon entries and avoid future crashes by ignoring unhandled png entries (resend)

[Akihiro Sagawa]

On Thu, 28 Aug 2014 09:30:34 +0300, Indrek Altpere wrote:
> Forgot to link to the bug before, also added more detailed description.
>
> Fixes https://bugs.winehq.org/show_bug.cgi?id=19241
>
> For the InnoSetup 5 crash (and likely other similar reported crashes), the
> issue seems to be GRPICONDIRENTRY with invalid information.

(...)
> As per MSDN blog, icon resources can contain raw PNG information instead of
> regular BITMAPINFO, but due to weird decisions, only way to differentiate
> between them is to check if the resource starts with PNG header bytes.
> http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
> Made the winemenubuilder ignore such entries for now (with fixme notices),
> since using PNG raw bytes as BITMAPINFO data is definitely invalid and is
> due to cause other issues/crashes when the best matching size icon happens
> to be PNG. A new bug should be opened for the missing PNG support.

Hello Indrek,

I think your patch looks nice, but needs to split into two or more parts.
This means one for invalid GRPICONDIRENTRY issue and one for PNG entry's
enhancement.
If you do this, it will be easier to read and easier to pass. For
details, please read our Patch Submission Guidelines[1], especially Code
Guidelines section.

[1] http://wiki.winehq.org/SubmittingPatches

Regards,
Akihiro Sagawa