Re: gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Paul Vriens]

Dmitry Timoshkov wrote:
> Hello,
> here is another version of the patch.
>
> According to test.winehq.ord data Windows 2000 behaves weird in some tests,
> this looks like a bug to me. This patch addresses these failures.
>
> Changelog:
>     gdi32: Make the bitmap test pass under Windows 2000.
>
> dlls/gdi32/tests/bitmap.c |   28 ++++++++++++++++------------
>  1 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/dlls/gdi32/tests/bitmap.c b/dlls/gdi32/tests/bitmap.c
> index f7bfdfd..ba4f8bd 100644
> --- a/dlls/gdi32/tests/bitmap.c
> +++ b/dlls/gdi32/tests/bitmap.c
> @@ -276,7 +276,7 @@ static void test_dib_info(HBITMAP hbm, const void *bits, const BITMAPINFOHEADER
>  {
>      BITMAP bm;
>      DIBSECTION ds;
> - INT ret, width_bytes;
> -
>
> INT ret, bm_width_bytes, dib_width_bytes;
> BYTE *buf;
>
> ret = GetObject(hbm, sizeof(bm), &bm);
>
> @@ -285,23 +285,25 @@ static void test_dib_info(HBITMAP hbm, const void *bits, const BITMAPINFOHEADER
>      ok(bm.bmType == 0, "wrong bm.bmType %d\n", bm.bmType);
>      ok(bm.bmWidth == bmih->biWidth, "wrong bm.bmWidth %d\n", bm.bmWidth);
>      ok(bm.bmHeight == bmih->biHeight, "wrong bm.bmHeight %d\n", bm.bmHeight);
> - width_bytes = DIB_GetWidthBytes(bm.bmWidth, bm.bmBitsPixel);
> - ok(bm.bmWidthBytes == width_bytes, "wrong bm.bmWidthBytes %d != %d\n", bm.bmWidthBytes, width_bytes);
> -
>
> dib_width_bytes = DIB_GetWidthBytes(bm.bmWidth, bm.bmBitsPixel);
> -
>
> bm_width_bytes = BITMAP_GetWidthBytes(bm.bmWidth, bm.bmBitsPixel);
> -
>
> if (bm.bmWidthBytes != dib_width_bytes) /* Win2k bug */
> -
>    ok(bm.bmWidthBytes == bm_width_bytes, "wrong bm.bmWidthBytes %d != %d\n", bm.bmWidthBytes, bm_width_bytes);
> -
>
> else
> -
>    ok(bm.bmWidthBytes == dib_width_bytes, "wrong bm.bmWidthBytes %d != %d\n", bm.bmWidthBytes, dib_width_bytes);
>
> ok(bm.bmPlanes == bmih->biPlanes, "wrong bm.bmPlanes %d\n", bm.bmPlanes);
>    ok(bm.bmBitsPixel == bmih->biBitCount, "bm.bmBitsPixel %d != %d\n", bm.bmBitsPixel, bmih->biBitCount);
>    ok(bm.bmBits == bits, "wrong bm.bmBits %p != %p\n", bm.bmBits, bits);
>
> buf = HeapAlloc(GetProcessHeap(), 0, bm.bmWidthBytes * bm.bmHeight + 4096);
> - width_bytes = BITMAP_GetWidthBytes(bm.bmWidth, bm.bmBitsPixel);
> - /* GetBitmapBits returns not 32-bit aligned data */
>    ret = GetBitmapBits(hbm, 0, NULL);
> - ok(ret == width_bytes * bm.bmHeight, "%d != %d\n", ret, width_bytes * bm.bmHeight);
> -
>
> ok(ret == bm_width_bytes * bm.bmHeight, "%d != %d\n", ret, bm_width_bytes * bm.bmHeight);
>
> memset(buf, 0xAA, bm.bmWidthBytes * bm.bmHeight + 4096);
> ret = GetBitmapBits(hbm, bm.bmWidthBytes * bm.bmHeight + 4096, buf);
> - ok(ret == width_bytes * bm.bmHeight, "%d != %d\n", ret, width_bytes * bm.bmHeight);
> -
>
> ok(ret == bm_width_bytes * bm.bmHeight, "%d != %d\n", ret, bm_width_bytes * bm.bmHeight);
>
> HeapFree(GetProcessHeap(), 0, buf);
>
> @@ -331,8 +333,9 @@ static void test_dib_info(HBITMAP hbm, const void *bits, const BITMAPINFOHEADER
>      ok(ret == sizeof(ds), "wrong size %d\n", ret);
>  ok(ds.dsBm.bmBits == bits, "wrong bm.bmBits %p != %p\n", ds.dsBm.bmBits, bits);
> - ok(ds.dsBmih.biSizeImage == ds.dsBm.bmWidthBytes * ds.dsBm.bmHeight, "%u != %u\n",
> -
>   ds.dsBmih.biSizeImage, ds.dsBm.bmWidthBytes * ds.dsBm.bmHeight);
> - if (ds.dsBm.bmWidthBytes != bm_width_bytes) /* Win2k bug */
> -
>    ok(ds.dsBmih.biSizeImage == ds.dsBm.bmWidthBytes * ds.dsBm.bmHeight, "%u != %u\n",
> -
>       ds.dsBmih.biSizeImage, ds.dsBm.bmWidthBytes * ds.dsBm.bmHeight);
> ok(bmih->biSizeImage == 0, "%u != 0\n", bmih->biSizeImage);
>    ds.dsBmih.biSizeImage = 0;
>
> @@ -968,14 +971,15 @@ static void test_bitmap(void)
>      assert(hdc != 0);
>  SetLastError(0xdeadbeef);
> - hbmp = CreateBitmap(0x7ffffff, 1, 1, 1, NULL);
> - ok(hbmp != 0, "CreateBitmap should not fail\n");
> - hbmp = CreateBitmap(0x7fff, 0x7fff, 1, 1, NULL);
> - ok(hbmp != 0, "CreateBitmap error %u\n", GetLastError());
> DeleteObject(hbmp);

Hi Dmitry,

This one will fail at least on my VMware box. Shouldn't the test (and it's 
confirmed by James and me) include something like:

ok(hbmp!=0 || (hbmp == 0 && GetLastError() == ERROR_NOT_ENOUGH_MEMORY), ....)

gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Reece Dunn]

2008/4/22 Paul Vriens :
> Hi Dmitry,
>
> This one will fail at least on my VMware box. Shouldn't the test (and it's
>  confirmed by James and me) include something like:
>
> ok(hbmp!=0 || (hbmp == 0 && GetLastError() == ERROR_NOT_ENOUGH_MEMORY), ....)

I would say something more like this:

ok( (hbmp != 0 && GetLastError() == ERROR_INVALID_PARAMETER) /* Win2K */
       || (hbmp == 0 && GetLastError() == ERROR_NOT_ENOUGH_MEMORY) /* XP */,
       ... )

Since the hbmp value and the GetLastError code are related together.

- Reece

gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Dmitry Timoshkov]

"Paul Vriens"  wrote:
> >  SetLastError(0xdeadbeef);
> > - hbmp = CreateBitmap(0x7ffffff, 1, 1, 1, NULL);
> > - ok(hbmp != 0, "CreateBitmap should not fail\n");
> > - hbmp = CreateBitmap(0x7fff, 0x7fff, 1, 1, NULL);
> > - ok(hbmp != 0, "CreateBitmap error %u\n", GetLastError());
> > DeleteObject(hbmp);
>
> Hi Dmitry,
>
> This one will fail at least on my VMware box. Shouldn't the test (and it's 
> confirmed by James and me) include something like:
>
> ok(hbmp!=0 || (hbmp == 0 && GetLastError() == ERROR_NOT_ENOUGH_MEMORY), ....)

Why will it fail? According to James (and you confirmed it):
> The maximum values for both width and height on win2k is 32767.
> Anything above that on either parameter returns
> ERROR_INVALID_PARAMETER.  Both params can be 32767 at the same time
> and the call will still succeed (assuming there's enough memory left).

gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Paul Vriens]

Dmitry Timoshkov wrote:
> "Paul Vriens"  wrote:
> > >  SetLastError(0xdeadbeef);
> > > - hbmp = CreateBitmap(0x7ffffff, 1, 1, 1, NULL);
> > > - ok(hbmp != 0, "CreateBitmap should not fail\n");
> > > - hbmp = CreateBitmap(0x7fff, 0x7fff, 1, 1, NULL);
> > > - ok(hbmp != 0, "CreateBitmap error %u\n", GetLastError());
> > > DeleteObject(hbmp);
> >
> > Hi Dmitry,
> >
> > This one will fail at least on my VMware box. Shouldn't the test (and 
> > it's confirmed by James and me) include something like:
> >
> > ok(hbmp!=0 || (hbmp == 0 && GetLastError() == 
> > ERROR_NOT_ENOUGH_MEMORY), ....)
>
> Why will it fail? According to James (and you confirmed it):
> > The maximum values for both width and height on win2k is 32767.
> > Anything above that on either parameter returns
> > ERROR_INVALID_PARAMETER.  Both params can be 32767 at the same time
> > and the call will still succeed (assuming there's enough memory left).

It's that last piece: "assuming there's enough memory left".

gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Dmitry Timoshkov]

"Paul Vriens"  wrote:
> > > This one will fail at least on my VMware box. Shouldn't the test (and 
> > > it's confirmed by James and me) include something like:
> > >
> > > ok(hbmp!=0 || (hbmp == 0 && GetLastError() == 
> > > ERROR_NOT_ENOUGH_MEMORY), ....)
> >
> > Why will it fail? According to James (and you confirmed it):
> > > The maximum values for both width and height on win2k is 32767.
> > > Anything above that on either parameter returns
> > > ERROR_INVALID_PARAMETER.  Both params can be 32767 at the same time
> > > and the call will still succeed (assuming there's enough memory left).
>
> It's that last piece: "assuming there's enough memory left".

That's a valid assumption I'd guess, and a lot of tests already rely
on this: every thing which indirectly or directly allocates memory,
creates windows, objects, processes, etc.

gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Paul Vriens]

Dmitry Timoshkov wrote:
> "Paul Vriens"  wrote:
> > > > This one will fail at least on my VMware box. Shouldn't the test 
> > > > (and it's confirmed by James and me) include something like:
> > > >
> > > > ok(hbmp!=0 || (hbmp == 0 && GetLastError() == 
> > > > ERROR_NOT_ENOUGH_MEMORY), ....)
> > >
> > > Why will it fail? According to James (and you confirmed it):
> > > > The maximum values for both width and height on win2k is 32767.
> > > > Anything above that on either parameter returns
> > > > ERROR_INVALID_PARAMETER.  Both params can be 32767 at the same time
> > > > and the call will still succeed (assuming there's enough memory left).
> >
> > It's that last piece: "assuming there's enough memory left".
>
> That's a valid assumption I'd guess, and a lot of tests already rely
> on this: every thing which indirectly or directly allocates memory,
> creates windows, objects, processes, etc.

But the test doesn't cater for that.

If I run the test with several heights on my win2k box the maximum I can have is:

CreateBitmap(0x7fff,0x2e63, ...

before it fails with ERROR_NOT_ENOUGH_MEMORY.

This of course differs on every run. My point is that ERROR_NOT_ENOUGH_MEMORY is 
totally valid and should be catered for in the test otherwise we would still 
have failures for this test.

gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Dmitry Timoshkov]

"Paul Vriens"  wrote:
> > > It's that last piece: "assuming there's enough memory left".
> >
> > That's a valid assumption I'd guess, and a lot of tests already rely
> > on this: every thing which indirectly or directly allocates memory,
> > creates windows, objects, processes, etc.
>
> But the test doesn't cater for that.
>
> If I run the test with several heights on my win2k box the maximum I can have is:
>
> CreateBitmap(0x7fff,0x2e63, ...
>
> before it fails with ERROR_NOT_ENOUGH_MEMORY.
>
> This of course differs on every run. My point is that ERROR_NOT_ENOUGH_MEMORY is 
> totally valid and should be catered for in the test otherwise we would still 
> have failures for this test.

I don't think it's a good idea to pollute every test that could fail
due to insufficient memory with fake error handling.

gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Paul Vriens]

Dmitry Timoshkov wrote:
> "Paul Vriens"  wrote:
> > > > It's that last piece: "assuming there's enough memory left".
> > >
> > > That's a valid assumption I'd guess, and a lot of tests already rely
> > > on this: every thing which indirectly or directly allocates memory,
> > > creates windows, objects, processes, etc.
> >
> > But the test doesn't cater for that.
> >
> > If I run the test with several heights on my win2k box the maximum I 
> > can have is:
> >
> > CreateBitmap(0x7fff,0x2e63, ...
> >
> > before it fails with ERROR_NOT_ENOUGH_MEMORY.
> >
> > This of course differs on every run. My point is that 
> > ERROR_NOT_ENOUGH_MEMORY is totally valid and should be catered for in 
> > the test otherwise we would still have failures for this test.
>
> I don't think it's a good idea to pollute every test that could fail
> due to insufficient memory with fake error handling.

The title of the patch says "Make the bitmap test pass under Windows 2000". 
Introducing the test as you did doesn't do that as in some cases it will fail.

But we can spent hours on this. Let's go on and see if AJ will accept the patch.

gdi32: Make the bitmap test pass under Windows 2000. Take 2

[Alexandre Julliard]

"Dmitry Timoshkov"  writes:
> "Paul Vriens"  wrote:
> > This of course differs on every run. My point is that ERROR_NOT_ENOUGH_MEMORY is 
> > totally valid and should be catered for in the test otherwise we would still 
> > have failures for this test.
>
> I don't think it's a good idea to pollute every test that could fail
> due to insufficient memory with fake error handling.

Obviously you don't want to do that for every test, but if you write a
test that requires 1Gb of memory then you definitely need to check, you
can't assume that this will always work.