[PATCH] comctl32: Always pass pcbRead to IStream_Read.

[Vincent Povirk]

Signed-off-by: Vincent Povirk 
---
pcbRead is a required argument. Windows Forms has an IStream
implementation that depends on it:

https://github.com/madewokherd/winforms/blob/36684d6dfd994f2a06dd6ef5fa73e01...

Found when testing Windows Double Explorer in Wine Mono.

dlls/comctl32/imagelist.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c
index 9a6ea3bbffc..ddf538d1336 100644
--- a/dlls/comctl32/imagelist.c
+++ b/dlls/comctl32/imagelist.c
@@ -2173,6 +2173,17 @@ ImageList_Merge (HIMAGELIST himl1, INT i1, HIMAGELIST himl2, INT i2,
 }

+static HRESULT stream_read(IStream *pstm, void *buf, ULONG bytestoread)
+{
+    HRESULT hr;
+    ULONG bytesread;
+
+    hr = IStream_Read(pstm, buf, bytestoread, &bytesread);
+    if (SUCCEEDED(hr) && bytesread != bytestoread)
+        hr = E_FAIL;
+    return hr;
+}
+
 /* helper for ImageList_Read, see comments below */
 static void *read_bitmap(IStream *pstm, BITMAPINFO *bmi)
 {
@@ -2180,13 +2191,13 @@ static void *read_bitmap(IStream *pstm, BITMAPINFO *bmi)
     int bitsperpixel, palspace;
     void *bits;

-    if (FAILED(IStream_Read ( pstm, &bmfh, sizeof(bmfh), NULL)))
+    if (FAILED(stream_read ( pstm, &bmfh, sizeof(bmfh))))
         return NULL;

if (bmfh.bfType != (('M'<<8)|'B'))
         return NULL;

-    if (FAILED(IStream_Read ( pstm, &bmi->bmiHeader, sizeof(bmi->bmiHeader), NULL)))
+    if (FAILED(stream_read ( pstm, &bmi->bmiHeader, sizeof(bmi->bmiHeader))))
         return NULL;

if ((bmi->bmiHeader.biSize != sizeof(bmi->bmiHeader)))
@@ -2205,13 +2216,13 @@ static void *read_bitmap(IStream *pstm, BITMAPINFO *bmi)
     bmi->bmiHeader.biSizeImage = get_dib_image_size( bmi );

/* read the palette right after the end of the bitmapinfoheader */
-    if (palspace && FAILED(IStream_Read(pstm, bmi->bmiColors, palspace, NULL)))
+    if (palspace && FAILED(stream_read(pstm, bmi->bmiColors, palspace)))
         return NULL;

bits = heap_alloc_zero(bmi->bmiHeader.biSizeImage);
     if (!bits) return NULL;

-    if (FAILED(IStream_Read(pstm, bits, bmi->bmiHeader.biSizeImage, NULL)))
+    if (FAILED(stream_read(pstm, bits, bmi->bmiHeader.biSizeImage)))
     {
         heap_free(bits);
         return NULL;
@@ -2263,7 +2274,7 @@ HIMAGELIST WINAPI ImageList_Read(IStream *pstm)

TRACE("%p\n", pstm);

-    if (FAILED(IStream_Read (pstm, &ilHead, sizeof(ILHEAD), NULL)))
+    if (FAILED(stream_read (pstm, &ilHead, sizeof(ILHEAD))))
    return NULL;
     if (ilHead.usMagic != (('L' << 8) | 'I'))
    return NULL;

[Zebediah Figura]

On 7/30/19 1:09 PM, Vincent Povirk wrote:
> Signed-off-by: Vincent Povirk
>
> pcbRead is a required argument. Windows Forms has an IStream
> implementation that depends on it:
>
> https://github.com/madewokherd/winforms/blob/36684d6dfd994f2a06dd6ef5fa73e01...

That's actually a bug in Mono, by my understanding. The CCW should be 
able to handle NULL return value pointers. I've sent a patch for it: 
https://github.com/mono/mono/pull/15906

[Nikolay Sivov]

On 7/30/19 9:16 PM, Zebediah Figura wrote:
> On 7/30/19 1:09 PM, Vincent Povirk wrote:
> > Signed-off-by: Vincent Povirk
> >
> > pcbRead is a required argument. Windows Forms has an IStream
> > implementation that depends on it:
> >
> > https://github.com/madewokherd/winforms/blob/36684d6dfd994f2a06dd6ef5fa73e01...
>
> That's actually a bug in Mono, by my understanding. The CCW should be 
> able to handle NULL return value pointers. I've sent a patch for it: 
> https://github.com/mono/mono/pull/15906

Testing comctl32 side of this was the first thing I did. Read() is 
called with NULL argument most of the time, for both version 5 and 
version 6.