[PATCH] comctl32/imagelist: fix ImageList_Read/Write.
Nikolay Sivov
nsivov at codeweavers.com
Wed Jun 20 02:49:57 CDT 2018
On 06/16/2018 07:19 PM, Denis Malikov wrote:
> From: Denis Malikov <mdn40000 at mail.ru>
> Date: Sat, 16 Jun 2018 16:35:45 +0700
> Subject: [PATCH] comctl32/imagelist: fix ImageList_Read/Write.
>
> Fix for versions x600 and x620 and pointer calculation for mixing image and mask bits.
>
> Tested on ReactOS 0.4.10 with *.reg files extracted from:
> - XP/2003 key HKCU\Software\Microsoft\Windows\CurrentVersion\Explorer\TrayNotify;
> - Vista/7 key HKEY_CLASSES_ROOT\Local Settings\Software\Microsoft\Windows\CurrentVersion\TrayNotify
>
> Signed-off-by: Denis Malikov <mdn40000 at mail.ru>
> ---
> dlls/comctl32/imagelist.c | 59 +++++++++++++++++++++++++++--------------------
> 1 file changed, 34 insertions(+), 25 deletions(-)
>
> diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c
> index a08d60752e..052d59f9e4 100644
> --- a/dlls/comctl32/imagelist.c
> +++ b/dlls/comctl32/imagelist.c
> @@ -59,7 +59,7 @@ struct _IMAGELIST
> INT cGrow; /* 0C: cGrow */
> INT cx; /* 10: cx */
> INT cy; /* 14: cy */
> - DWORD x4;
> + DWORD x4; /* hack for IL from stream. Keep version here */
> UINT flags; /* 1C: flags */
> COLORREF clrFg; /* 20: foreground color */
> COLORREF clrBk; /* 24: background color */
> @@ -83,6 +83,7 @@ struct _IMAGELIST
> };
The idea is to keep this structure binary compatible, that's why it
starts with a part with specific layout.
If you want to store source stream version, append a new field for that.
I actually like the idea of using
source stream format version to save the list back, it seems like a good
compromise.
>
> #define IMAGELIST_MAGIC 0x53414D58
> +#define IMAGELIST_VERSION 0x101
>
> /* Header used by ImageList_Read() and ImageList_Write() */
> #include "pshpack2.h"
> @@ -806,6 +807,7 @@ ImageList_Create (INT cx, INT cy, UINT flags,
> himl->clrFg = CLR_DEFAULT;
> himl->clrBk = CLR_NONE;
> himl->color_table_set = FALSE;
> + himl->x4 = 0;
>
> /* initialize overlay mask indices */
> for (nCount = 0; nCount < MAX_OVERLAYIMAGE; nCount++)
> @@ -2257,38 +2259,43 @@ HIMAGELIST WINAPI ImageList_Read(IStream *pstm)
> BITMAPINFO *image_info = (BITMAPINFO *)image_buf;
> BITMAPINFO *mask_info = (BITMAPINFO *)mask_buf;
> void *image_bits, *mask_bits = NULL;
> - ILHEAD ilHead;
> - HIMAGELIST himl;
> + ILHEAD ilHead;
> + HIMAGELIST himl;
> unsigned int i;
>
> TRACE("%p\n", pstm);
>
> if (FAILED(IStream_Read (pstm, &ilHead, sizeof(ILHEAD), NULL)))
> - return NULL;
> + return NULL;
> if (ilHead.usMagic != (('L' << 8) | 'I'))
> - return NULL;
> - if (ilHead.usVersion != 0x101) /* probably version? */
> - return NULL;
> + return NULL;
No need to reformat existing lines.
> + if (ilHead.usVersion != IMAGELIST_VERSION &&
> + ilHead.usVersion != 0x600 && /* XP/2003 version */
> + ilHead.usVersion != 0x620) /* Vista/7 version */
> + return NULL;
The question is if there's any format differences between 0x600 and
0x620, it was probably bumped for a reason.
>
> TRACE("cx %u, cy %u, flags 0x%04x, cCurImage %u, cMaxImage %u\n",
> ilHead.cx, ilHead.cy, ilHead.flags, ilHead.cCurImage, ilHead.cMaxImage);
>
> himl = ImageList_Create(ilHead.cx, ilHead.cy, ilHead.flags, ilHead.cCurImage, ilHead.cMaxImage);
> if (!himl)
> - return NULL;
> + return NULL;
> +
> + // keep version from stream
> + himl->x4 = ilHead.usVersion;
No cpp-style comments, please.
>
> @@ -3078,10 +3087,10 @@ BOOL WINAPI ImageList_Write(HIMAGELIST himl, IStream *pstm)
> TRACE("%p %p\n", himl, pstm);
>
> if (!is_valid(himl))
> - return FALSE;
> + return FALSE;
>
> ilHead.usMagic = (('L' << 8) | 'I');
> - ilHead.usVersion = 0x101;
> + ilHead.usVersion = himl->x4 > 0 ? himl->x4 : IMAGELIST_VERSION;
> ilHead.cCurImage = himl->cCurImage;
> ilHead.cMaxImage = himl->cMaxImage;
> ilHead.cGrow = himl->cGrow;
> @@ -3090,23 +3099,23 @@ BOOL WINAPI ImageList_Write(HIMAGELIST himl, IStream *pstm)
> ilHead.bkcolor = himl->clrBk;
> ilHead.flags = himl->flags;
> for(i = 0; i < 4; i++) {
> - ilHead.ovls[i] = himl->nOvlIdx[i];
> + ilHead.ovls[i] = himl->nOvlIdx[i];
> }
>
> TRACE("cx %u, cy %u, flags 0x04%x, cCurImage %u, cMaxImage %u\n",
> ilHead.cx, ilHead.cy, ilHead.flags, ilHead.cCurImage, ilHead.cMaxImage);
>
> if(FAILED(IStream_Write(pstm, &ilHead, sizeof(ILHEAD), NULL)))
> - return FALSE;
> + return FALSE;
>
> /* write the bitmap */
> if(!_write_bitmap(himl->hbmImage, pstm))
> - return FALSE;
> + return FALSE;
>
> /* write the mask if we have one */
> if(himl->flags & ILC_MASK) {
> - if(!_write_bitmap(himl->hbmMask, pstm))
> - return FALSE;
> + if(!_write_bitmap(himl->hbmMask, pstm))
> + return FALSE;
> }
Doesn't this write in old format, with mismatching version field?
More information about the wine-devel
mailing list