Try #3 for IDirectDrawSurface_GetSurfaceDesc error checking in ddraw test

[chris ahrendt]

Michael / All

here is the third go at the ddraw test... and the patch for that...

note I went ahead and used the goto.

Also I found an error in the test where it was allocating the buffer in 
one place and it did not free it.

Now here is the curious thing...  When I did the first run of the ddraw 
test I got the output found in the no_ulimit_run attachment which did 
not seem right as nothing ran or didnt seem to.
So I did the ulimit -s unlimited  and then re-ran the test and got the 
second output...

Now shouldn't the test run the same reguardless of the stack limit?

Chris

Try #3 for IDirectDrawSurface_GetSurfaceDesc error checking in ddraw test

[Michael Karcher]

Am Mittwoch, den 15.10.2008, 17:25 -0700 schrieb chris ahrendt:
> Michael / All
>
> here is the third go at the ddraw test... and the patch for that...

Looks much better now, but two issues remains.
> note I went ahead and used the goto.

As I suggested the approach, this is OK with me, but...
> @@ -2175,6 +2208,11 @@ static void SizeTest(void)
>      desc.dwHeight = 128;
>      ret = IDirectDraw_CreateSurface(lpDD, &desc, &dsurface, NULL);
>      ok(ret == DDERR_INVALIDPARAMS, "Creating an offscreen plain surface without width info returned
> - if (FAILED(ret))
> -
>  {
> -
>   skip("Can't create cubemap surface\n");
> -
>   goto err;
> -
>  }
> if(dsurface)
>    {
>        IDirectDrawSurface_Release(dsurface);

In your last try you had no buggy "fixes" like this one. Please take a
look at the ok() statement one line abve your insertion to find out what
Wine expects hr to be. BTW: the skip message is also wrong, but that is
a minor nitpick, as this hunk is completely wrong (shouldn't be there at
all).
> Also I found an error in the test where it was allocating the buffer in 
> one place and it did not free it.

You probably mean
> @@ -1328,6 +1332,12 @@ static void AttachmentTest(void)
>      ddsd.ddsCaps.dwCaps = DDSCAPS_PRIMARYSURFACE | DDSCAPS_BACKBUFFER;
>      hr = IDirectDraw_CreateSurface(lpDD, &ddsd, &surface2, NULL);
>      ok(hr==DDERR_INVALIDCAPS,"CreateSurface returned: %x\n",hr);
> - if (FAILED(hr)) {
> -
>    skip("failed to create surface2\n");
> -
>    goto err;
> -
>   }
> - IDirectDrawSurface_Release(surface2);
> - /* This old ddraw version happily creates explicit front buffers */
> memset(&ddsd, 0, sizeof(ddsd));
> ddsd.dwSize = sizeof(ddsd);

Same thing applies as for the last hunk (except for the skip message
that is right here). Please double-check what is expected before you
conclude that FAILED(hr) means the test has to be aborted.
> Now here is the curious thing...  When I did the first run of the ddraw 
> test I got the output found in the no_ulimit_run attachment which did 
> not seem right as nothing ran or didnt seem to.

make test runs many testcases in sucession. Please like at the runtest
lines in your quoted output. In this case, the first test failed, as in
line 1138 the ATI driver did not comply to the surface creation
request.
> So I did the ulimit -s unlimited  and then re-ran the test and got the 
> second output...

In this case, the dsurface, overlay and refcount tests succeeded, and
the visual test got started. This test crashed (and would do that on
your system also before your patches, as it is completely unrelated).
> Now shouldn't the test run the same reguardless of the stack limit?

Not necessarily. The problem is that the ATI driver behaves differently
with different stack limits, and the Wine developers have to decide
whether they accept both behaviours as OK.

Regards,
  Michael Karcher

Try #3 for IDirectDrawSurface_GetSurfaceDesc error checking in ddraw test

[Roderick Colenbrander]

Hi,

Please use a consistent indenting style. Use the same one as used by the rest of the code and make sure not to mix up spaces and tabs.

Roderick
> Michael / All
>
> here is the third go at the ddraw test... and the patch for that...
>
> note I went ahead and used the goto.
>
> Also I found an error in the test where it was allocating the buffer in 
> one place and it did not free it.
>
> Now here is the curious thing...  When I did the first run of the ddraw 
> test I got the output found in the no_ulimit_run attachment which did 
> not seem right as nothing ran or didnt seem to.
> So I did the ulimit -s unlimited  and then re-ran the test and got the 
> second output...
>
> Now shouldn't the test run the same reguardless of the stack limit?
>
> Chris