Re: [PATCH] gdiplus: Fix test crash in GdipSaveImageToStream.

[Lei Zhang]

On Tue, Mar 11, 2008 at 10:26 PM, Nathan Beckmann
<nathan.beckmann(a)gmail.com> wrote:
> Correctly initialized parameters to GetDIBits. (Based on
>  GdipLockBitmapBits.)
>
> Testing seems to indicate that the error doesn't occur on Debian, but
>  shows up in Slackware. Valgrind also complains.
>  ---
>   dlls/gdiplus/image.c |   28 ++++++++++++++++++++++------
>   1 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c
>  index 8307938..ad15148 100644
>  --- a/dlls/gdiplus/image.c
>  +++ b/dlls/gdiplus/image.c
>  @@ -908,7 +908,9 @@ GpStatus WINGDIPAPI GdipSaveImageToStream(GpImage *image, IStream* stream,
>      HRESULT hr;
>      short type;
>      HBITMAP hbmp;
>  +    HBITMAP old_hbmp;
>      HDC hdc;
>  +    int bm_is_selected;
>      BITMAPINFO bmp_info;
>      LPVOID bmp_bits;
>      encode_image_func* encode_image;
>  @@ -917,6 +919,7 @@ GpStatus WINGDIPAPI GdipSaveImageToStream(GpImage *image, IStream* stream,
>      unsigned int dummy;
>      int i;
>
> +    old_hbmp = 0;
>      output = NULL;
>      output_size = 0;
>
> @@ -943,18 +946,32 @@ GpStatus WINGDIPAPI GdipSaveImageToStream(GpImage *image, IStream* stream,
>      hr = IPicture_get_Handle(image->picture, (OLE_HANDLE*)&hbmp);
>      if (FAILED(hr) || !hbmp)
>          return GenericError;
>  +    hr = IPicture_get_CurDC(image->picture, &hdc);
>  +    if (FAILED(hr))
>  +        return GenericError;
>  +    bm_is_selected = (hdc != 0);
>  +    if (!bm_is_selected) {
>  +        hdc = CreateCompatibleDC(0);
>  +        old_hbmp = SelectObject(hdc, hbmp);
>  +    }
>
> /* get bits from HBITMAP */
>  -    hdc = GetDC(0);
>      bmp_info.bmiHeader.biSize = sizeof(bmp_info.bmiHeader);
>  +    bmp_info.bmiHeader.biBitCount = 0;
>      GetDIBits(hdc, hbmp, 0, 0, NULL, &bmp_info, DIB_RGB_COLORS);
>
> bmp_bits = GdipAlloc(bmp_info.bmiHeader.biSizeImage);
>  -    if (!bmp_bits) {
>  -        ReleaseDC(0, hdc);
>  -        return OutOfMemory;
>  +
>  +    if (bmp_bits)
>  +        GetDIBits(hdc, hbmp, 0, abs(bmp_info.bmiHeader.biHeight), bmp_bits, &bmp_info, DIB_RGB_COLORS);
>  +
>  +    if (!bm_is_selected) {
>  +        SelectObject(hdc, old_hbmp);
>  +        DeleteDC(hdc);
>      }
>  -    GetDIBits(hdc, hbmp, 0, bmp_info.bmiHeader.biHeight, bmp_bits, &bmp_info, DIB_RGB_COLORS);
>  +
>  +    if (!bmp_bits)
>  +        return OutOfMemory;
>
> stat = encode_image(bmp_bits, &bmp_info, &output, &output_size);
>      if (stat == Ok)
>  @@ -962,7 +979,6 @@ GpStatus WINGDIPAPI GdipSaveImageToStream(GpImage *image, IStream* stream,
>
> GdipFree(output);
>      GdipFree(bmp_bits);
>  -    ReleaseDC(0, hdc);
>
> return stat;
>   }
>  --
>  1.5.4.2

FYI, here's the valgrind log:

http://www.kegel.com/wine/valgrind/logs-2008-03-11/vg-gdiplus_image-diff.txt

We're passing an uninitialized bmp_info into GetDIBits(), but somehow
it magically works on Debian.