Ok I have threaded through ddraw_test adding as I had them fail a check and a fix in dsurface.c test. The test now fails when the CreateSurface fails. Before this there were several point in the test where the return status was not checked. As I encountered them and they failed with an exception I added the failure I found at other parts which did the create surface. For some reason a 3rd and 4th surface fails on ATI.
Addached you will find the proposed fix. I also found a point where the code did not skip but did a goto to the error point with the variables unitialised.
Chris
From 52071bddae683a1964d64be0acbfec8d7b2c2c5e Mon Sep 17 00:00:00 2001
From: Chris Ahrendt celticht32@aol.com Date: Sun, 12 Oct 2008 21:44:21 -0400 Subject: [PATCH] Fix to ddtest to fail test if IDirectDraw_CreateSurface failed. Create Surface was failing and setting the handle to the surface to null. So when the free / release was called it threw an exception instead of failing the test.
--- dlls/ddraw/tests/dsurface.c | 92 ++++++++++++++++++++++++++++++++++--------- 1 files changed, 73 insertions(+), 19 deletions(-)
diff --git a/dlls/ddraw/tests/dsurface.c b/dlls/ddraw/tests/dsurface.c index fc7bb88..4f545d9 100644 --- a/dlls/ddraw/tests/dsurface.c +++ b/dlls/ddraw/tests/dsurface.c @@ -198,7 +198,10 @@ static void MipMapCreationTest(void) ddsd.dwHeight = 32; rc = IDirectDraw_CreateSurface(lpDD, &ddsd, &lpDDSMipMapTest, NULL); ok(rc==DDERR_INVALIDPARAMS,"CreateSurface returned: %x\n",rc); - + if (FAILED(rc)) { + skip("failed to create surface\n"); + return; + } /* Destroy the surface. */ if( rc == DD_OK ) IDirectDrawSurface_Release(lpDDSMipMapTest); @@ -1004,7 +1007,10 @@ static void EnumTest(void) ddsd.dwHeight = 32; rc = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface, NULL); ok(rc==DD_OK,"CreateSurface returned: %x\n",rc); - + if (FAILED(rc)) { + skip("failed to create surface\n"); + return; + } memset(&ctx, 0, sizeof(ctx)); ctx.expected[0] = surface; rc = IDirectDrawSurface_GetAttachedSurface(ctx.expected[0], &ddsd.ddsCaps, &ctx.expected[1]); @@ -1055,7 +1061,10 @@ static void AttachmentTest7(void) ddsd.dwHeight = 128; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface1, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface\n"); + return; + } /* ROOT */ num = 0; IDirectDrawSurface7_EnumAttachedSurfaces(surface1, &num, SurfaceCounter); @@ -1092,7 +1101,10 @@ static void AttachmentTest7(void) ddsd.dwHeight = 16; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface2, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface2\n"); + return; + } hr = IDirectDrawSurface7_AddAttachedSurface(surface1, surface2); ok(hr == DDERR_CANNOTATTACHSURFACE, "Attaching a 16x16 surface to a 128x128 texture root returned %08x\n", hr); hr = IDirectDrawSurface7_AddAttachedSurface(surface2, surface1); @@ -1112,7 +1124,10 @@ static void AttachmentTest7(void) ddsd.dwHeight = 16; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface2, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface\n"); + return; + } hr = IDirectDrawSurface7_AddAttachedSurface(surface1, surface2); ok(hr == DDERR_CANNOTATTACHSURFACE, "Attaching a 16x16 offscreen plain surface to a 128x128 texture root returned %08x\n", hr); hr = IDirectDrawSurface7_AddAttachedSurface(surface2, surface1); @@ -1136,7 +1151,10 @@ static void AttachmentTest7(void) ddsd.dwBackBufferCount = 2; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface1, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface\n"); + return; + } num = 0; IDirectDrawSurface7_EnumAttachedSurfaces(surface1, &num, SurfaceCounter); ok(num == 1, "Primary surface has %d surfaces attached, expected 1\n", num); @@ -1149,13 +1167,20 @@ static void AttachmentTest7(void) ddsd.ddsCaps.dwCaps = DDSCAPS_PRIMARYSURFACE | DDSCAPS_FRONTBUFFER; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface1, NULL); ok(hr==DDERR_INVALIDCAPS,"CreateSurface returned: %x\n",hr); + if (FAILED(hr)) { + skip("failed to create surface1\n"); + return; + } memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); ddsd.dwFlags = DDSD_CAPS; ddsd.ddsCaps.dwCaps = DDSCAPS_PRIMARYSURFACE | DDSCAPS_BACKBUFFER; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface2, NULL); ok(hr==DDERR_INVALIDCAPS,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface2\n"); + return; + } /* Try a single primary and two offscreen plain surfaces */ memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); @@ -1163,7 +1188,10 @@ static void AttachmentTest7(void) ddsd.ddsCaps.dwCaps = DDSCAPS_PRIMARYSURFACE; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface1, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface1\n"); + return; + } memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); ddsd.dwFlags = DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT; @@ -1172,7 +1200,10 @@ static void AttachmentTest7(void) ddsd.dwHeight = GetSystemMetrics(SM_CYSCREEN); hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface2, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface2\n"); + return; + } memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); ddsd.dwFlags = DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT; @@ -1181,7 +1212,10 @@ static void AttachmentTest7(void) ddsd.dwHeight = GetSystemMetrics(SM_CYSCREEN); hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface3, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface3\n"); + return; + } /* This one has a different size */ memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); @@ -1191,6 +1225,10 @@ static void AttachmentTest7(void) ddsd.dwHeight = 128; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface4, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); + if (FAILED(hr)) { + skip("failed to create surface4\n"); + return; + }
hr = IDirectDrawSurface7_AddAttachedSurface(surface1, surface2); ok(hr == DDERR_CANNOTATTACHSURFACE, "Attaching an offscreen plain surface to a front buffer returned %08x\n", hr); @@ -1334,7 +1372,7 @@ static void AttachmentTest(void) ddsd.dwFlags = DDSD_CAPS; ddsd.ddsCaps.dwCaps = DDSCAPS_PRIMARYSURFACE | DDSCAPS_FRONTBUFFER; hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface1, NULL); - ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); + ok(hr==DD_OK,"CreateSurface return ed: %x\n",hr); IDirectDrawSurface_Release(surface1);
/* Try a single primary and two offscreen plain surfaces */ @@ -1344,7 +1382,10 @@ static void AttachmentTest(void) ddsd.ddsCaps.dwCaps = DDSCAPS_PRIMARYSURFACE; hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface1, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface1\n"); + return; + } memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); ddsd.dwFlags = DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT; @@ -1353,7 +1394,10 @@ static void AttachmentTest(void) ddsd.dwHeight = GetSystemMetrics(SM_CYSCREEN); hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface2, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface2\n"); + return; + } memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); ddsd.dwFlags = DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT; @@ -1362,7 +1406,10 @@ static void AttachmentTest(void) ddsd.dwHeight = GetSystemMetrics(SM_CYSCREEN); hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface3, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface3\n"); + return; + } /* This one has a different size */ memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); @@ -1372,7 +1419,10 @@ static void AttachmentTest(void) ddsd.dwHeight = 128; hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface4, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface4\n"); + return; + } hr = IDirectDrawSurface_AddAttachedSurface(surface1, surface2); ok(hr == DD_OK, "Attaching an offscreen plain surface to a front buffer returned %08x\n", hr); /* Try the reverse without detaching first */ @@ -2208,6 +2258,10 @@ static void SizeTest(void) desc.dwWidth = 128; /* Keep them set to check what happens */ ret = IDirectDraw_CreateSurface(lpDD, &desc, &dsurface, NULL); ok(ret == DD_OK, "Creating a primary surface without width and height info returned %08x\n", ret); + if (FAILED(ret)) { + skip("failed to create surface\n"); + return; + } if(dsurface) { ret = IDirectDrawSurface_GetSurfaceDesc(dsurface, &desc); @@ -2491,9 +2545,9 @@ static void PaletteTest(void) hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &lpSurf, NULL); ok(hr==DD_OK, "CreateSurface returned: %x\n",hr); if (FAILED(hr)) { - skip("failed to create surface\n"); - goto err; - } + skip("failed to create surface\n"); + return; + }
hr = IDirectDrawSurface_SetPalette(lpSurf, palette); ok(hr == DDERR_INVALIDPIXELFORMAT, "CreateSurface returned: %x\n",hr); @@ -2503,7 +2557,7 @@ static void PaletteTest(void)
hr = IDirectDrawSurface_GetPalette(lpSurf, &palette); ok(hr == DDERR_NOPALETTEATTACHED, "CreateSurface returned: %x\n",hr); - + err:
if (lpSurf) IDirectDrawSurface_Release(lpSurf);
On Sun, Oct 12, 2008 at 8:52 PM, Chris Ahrendt celticht32@aol.com wrote:
Ok I have threaded through ddraw_test adding as I had them fail a check and a fix in dsurface.c test. The test now fails when the CreateSurface fails. Before this there were several point in the test where the return status was not checked. As I encountered them and they failed with an exception I added the failure I found at other parts which did the create surface. For some reason a 3rd and 4th surface fails on ATI.
Addached you will find the proposed fix. I also found a point where the code did not skip but did a goto to the error point with the variables unitialised.
@@ -198,7 +198,10 @@ static void MipMapCreationTest(void) ddsd.dwHeight = 32; rc = IDirectDraw_CreateSurface(lpDD, &ddsd, &lpDDSMipMapTest, NULL); ok(rc==DDERR_INVALIDPARAMS,"CreateSurface returned: %x\n",rc); - + if (FAILED(rc)) { + skip("failed to create surface\n"); + return; + }
Can you see why this is wrong? You've done this in a couple other places.
On Sun, Oct 12, 2008 at 09:52:06PM -0400, Chris Ahrendt wrote:
Ok I have threaded through ddraw_test adding as I had them fail a check and a fix in dsurface.c test. The test now fails when the CreateSurface fails. Before this there were several point in the test where the return status was not checked. As I encountered them and they failed with an exception I added the failure I found at other parts which did the create surface. For some reason a 3rd and 4th surface fails on ATI.
Addached you will find the proposed fix. I also found a point where the code did not skip but did a goto to the error point with the variables unitialised.
Chris
From 52071bddae683a1964d64be0acbfec8d7b2c2c5e Mon Sep 17 00:00:00 2001
From: Chris Ahrendt celticht32@aol.com Date: Sun, 12 Oct 2008 21:44:21 -0400 Subject: [PATCH] Fix to ddtest to fail test if IDirectDraw_CreateSurface failed. Create Surface was failing and setting the handle to the surface to null. So when the free / release was called it threw an exception instead of failing the test.
ok(rc==DDERR_INVALIDPARAMS,"CreateSurface returned: %x\n",rc);
- if (FAILED(rc)) {
skip("failed to create surface\n");return;}
here it is _expected_ that the surface creation fails. So this code is not useful. ALso in other places.
ciao, Marcus
Marcus Meissner wrote:
On Sun, Oct 12, 2008 at 09:52:06PM -0400, Chris Ahrendt wrote:
Ok I have threaded through ddraw_test adding as I had them fail a check and a fix in dsurface.c test. The test now fails when the CreateSurface fails. Before this there were several point in the test where the return status was not checked. As I encountered them and they failed with an exception I added the failure I found at other parts which did the create surface. For some reason a 3rd and 4th surface fails on ATI.
Addached you will find the proposed fix. I also found a point where the code did not skip but did a goto to the error point with the variables unitialised.
Chris
From 52071bddae683a1964d64be0acbfec8d7b2c2c5e Mon Sep 17 00:00:00 2001
From: Chris Ahrendt celticht32@aol.com Date: Sun, 12 Oct 2008 21:44:21 -0400 Subject: [PATCH] Fix to ddtest to fail test if IDirectDraw_CreateSurface failed. Create Surface was failing and setting the handle to the surface to null. So when the free / release was called it threw an exception instead of failing the test.
ok(rc==DDERR_INVALIDPARAMS,"CreateSurface returned: %x\n",rc);
- if (FAILED(rc)) {
skip("failed to create surface\n");return;}here it is _expected_ that the surface creation fails. So this code is not useful. ALso in other places.
ciao, Marcus
Ok that I can fix easy enough...
chris
ok(rc==DDERR_INVALIDPARAMS,"CreateSurface returned: %x\n",rc);
- if (FAILED(rc)) {
skip("failed to create surface\n");return;}here it is _expected_ that the surface creation fails. So this code is not useful. ALso in other places.
ciao, Marcus
Ok that I can fix easy enough...
chris
Ok I fixed the code to take this out... however : one section there is something I am not sure of so I left it in to prevent the exception from occuring.
here is my change that doesn't cause an exception:
ZeroMemory(&desc, sizeof(desc)); desc.dwSize = sizeof(desc); desc.dwFlags = DDSD_CAPS; desc.ddsCaps.dwCaps |= DDSCAPS_PRIMARYSURFACE; desc.dwHeight = 128; /* Keep them set to check what happens */ desc.dwWidth = 128; /* Keep them set to check what happens */ ret = IDirectDraw_CreateSurface(lpDD, &desc, &dsurface, NULL); ok(ret == DD_OK, "Creating a primary surface without width and height info returned %08x\n", ret);
/* This should be handled but CreateSurface is not returning right */ if (FAILED(ret)) { skip("Can't create cubemap surface\n"); return; }
if(dsurface) { ret = IDirectDrawSurface_GetSurfaceDesc(dsurface, &desc); ok(ret == DD_OK, "GetSurfaceDesc returned %x\n", ret);
If those lines are not there then when it gets to the ret = IDirectDrawSurface_GetSurfaceDesc(dsurface, &desc); line it throws and exception because dsurface is null.. so I am wondering if the if should be if(&dsurface) instead of the above?
Chris
From 6efc80d39533cd5940265b752442f607941fbab9 Mon Sep 17 00:00:00 2001
From: Chris Ahrendt celticht32@aol.com Date: Mon, 13 Oct 2008 14:23:30 -0400 Subject: [PATCH] Pared Down Test For ddraw test... fixes error with IDirectDraw_CreateSurface where the test is not checking the return from the call. CreateSurface is returning a null.
--- dlls/ddraw/tests/dsurface.c | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/dlls/ddraw/tests/dsurface.c b/dlls/ddraw/tests/dsurface.c index fc7bb88..21e6b15 100644 --- a/dlls/ddraw/tests/dsurface.c +++ b/dlls/ddraw/tests/dsurface.c @@ -1181,7 +1181,10 @@ static void AttachmentTest7(void) ddsd.dwHeight = GetSystemMetrics(SM_CYSCREEN); hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface3, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface3\n"); + return; + } /* This one has a different size */ memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); @@ -1191,7 +1194,10 @@ static void AttachmentTest7(void) ddsd.dwHeight = 128; hr = IDirectDraw7_CreateSurface(dd7, &ddsd, &surface4, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface4\n"); + return; + } hr = IDirectDrawSurface7_AddAttachedSurface(surface1, surface2); ok(hr == DDERR_CANNOTATTACHSURFACE, "Attaching an offscreen plain surface to a front buffer returned %08x\n", hr); hr = IDirectDrawSurface7_AddAttachedSurface(surface2, surface1); @@ -1362,7 +1368,10 @@ static void AttachmentTest(void) ddsd.dwHeight = GetSystemMetrics(SM_CYSCREEN); hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface3, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface3\n"); + return; + } /* This one has a different size */ memset(&ddsd, 0, sizeof(ddsd)); ddsd.dwSize = sizeof(ddsd); @@ -1372,7 +1381,10 @@ static void AttachmentTest(void) ddsd.dwHeight = 128; hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface4, NULL); ok(hr==DD_OK,"CreateSurface returned: %x\n",hr); - + if (FAILED(hr)) { + skip("failed to create surface4\n"); + return; + } hr = IDirectDrawSurface_AddAttachedSurface(surface1, surface2); ok(hr == DD_OK, "Attaching an offscreen plain surface to a front buffer returned %08x\n", hr); /* Try the reverse without detaching first */ @@ -2208,6 +2220,12 @@ static void SizeTest(void) desc.dwWidth = 128; /* Keep them set to check what happens */ ret = IDirectDraw_CreateSurface(lpDD, &desc, &dsurface, NULL); ok(ret == DD_OK, "Creating a primary surface without width and height info returned %08x\n", ret); + /* This should be handled but CreateSurface is failing */ + if (FAILED(ret)) + { + skip("Can't create cubemap surface\n"); + return; + } if(dsurface) { ret = IDirectDrawSurface_GetSurfaceDesc(dsurface, &desc); @@ -2492,7 +2510,7 @@ static void PaletteTest(void) ok(hr==DD_OK, "CreateSurface returned: %x\n",hr); if (FAILED(hr)) { skip("failed to create surface\n"); - goto err; + return; }
hr = IDirectDrawSurface_SetPalette(lpSurf, palette);
Am Montag, den 13.10.2008, 14:30 -0400 schrieb Chris Ahrendt:
Ok I fixed the code to take this out... however : one section there is something I am not sure of so I left it in to prevent the exception from occuring.
This is an additional explanation in addition to the remarks I made to your "Try #2" mail.
here is my change that doesn't cause an exception:
ret = IDirectDraw_CreateSurface(lpDD, &desc, &dsurface, NULL); ok(ret == DD_OK, "Creating a primary surface without width andheight info returned %08x\n", ret);
/* This should be handled but CreateSurface is not returning right */ if (FAILED(ret)) { skip("Can't create cubemap surface\n"); return; }
if(dsurface) { ret = IDirectDrawSurface_GetSurfaceDesc(dsurface, &desc); ok(ret == DD_OK, "GetSurfaceDesc returned %x\n", ret);If those lines are not there then when it gets to the ret = IDirectDrawSurface_GetSurfaceDesc(dsurface, &desc); line it throws and exception because dsurface is null.. so I am wondering if the if should be if(&dsurface) instead of the above?
Sorry, you seem to misunderstand the C language in this regard. Code flow never gets to the GetSurfaceDesc call if dsurface is NULL, because if(dsurface) means the same as if(dsurface != NULL) so the whole block is only executed if dsurface is not NULL.
You definitely do not want "if(&dsurface)" instead because that would test if the address of the variable dsurface (that is the memory location where the pointer to the interface is stored) is not NULL. This is *always* the case, as the variable "dsurface" is allocated on the stack and &dsurface is thus pointing into the stack. The check if(dsurface) checks whether the contents of the variable dsurface, which should point to the DirectDrawSurface interface, is a NULL value pointing nowhere instead, which is exactly what we need here.
Regards, Michael Karcher
-
Chris Ahrendt -
James Hawkins -
Marcus Meissner -
Michael Karcher