Re: [PATCH 1/4] ddraw/tests: New CreateSurface for DDSCAPS_BACKBUFFER tests.

[Stefan Dösinger]

Am 22.07.2010 um 21:13 schrieb Oldřich Jedlička:
> +    hr = IDirectDraw_QueryInterface(lpDD, &IID_IDirectDraw, (void **) &dd);
> +    ok(SUCCEEDED(hr), "IDirectDraw_QueryInterface failed: 0x%08x\n", hr);
> This is redundant, lpDD is already an IDirectDraw interface.
> +    hr = IDirectDraw_CreateSurface(dd, &ddsd, &surf, NULL);
> +    ok(SUCCEEDED(hr), "IDirectDraw_CreateSurface failed: 0x%08x\n", hr);
> This test will fail before your 3/4 patch is in, so you'll have to use todo_wine for now, and remove in the patch that implements the correct behavior. You'll have to handle the NULL pointer that is returned in case of a failure.

[PATCH 1/4] ddraw/tests: New CreateSurface for DDSCAPS_BACKBUFFER tests.

[Oldřich Jedlička]

Hi Stefan,

On Thursday 22 July 2010 22:05:21 Stefan Dösinger wrote:
> Am 22.07.2010 um 21:13 schrieb Oldřich Jedlička:
> > +    hr = IDirectDraw_QueryInterface(lpDD, &IID_IDirectDraw, (void **)
> > &dd); +    ok(SUCCEEDED(hr), "IDirectDraw_QueryInterface failed:
> > 0x%08x\n", hr);
>
> This is redundant, lpDD is already an IDirectDraw interface.

You are right, I thought it is some default (=latest, V7) interface. I will 
change it.
> > +    hr = IDirectDraw_CreateSurface(dd, &ddsd, &surf, NULL);
> > +    ok(SUCCEEDED(hr), "IDirectDraw_CreateSurface failed: 0x%08x\n", hr);
>
> This test will fail before your 3/4 patch is in, so you'll have to use
> todo_wine for now, and remove in the patch that implements the correct
> behavior. You'll have to handle the NULL pointer that is returned in case
> of a failure.

The same is for my 2/4. I will add the todo_wine in 1/4 and 2/4 and remove 
them in 3/4 and 4/4.

Which NULL pointer do you mean? Does the todo_wine mean that the test reports 
problem and just continues (so that surf is NULL)?

Thanks,
Oldrich.

[PATCH 1/4] ddraw/tests: New CreateSurface for DDSCAPS_BACKBUFFER tests.

[Stefan Dösinger]

Am 22.07.2010 um 22:15 schrieb Oldřich Jedlička:
> > This is redundant, lpDD is already an IDirectDraw interface.
> > You are right, I thought it is some default (=latest, V7) interface. I will 
> > change it.
> > Fyi:
> > static LPDIRECTDRAW lpDD = NULL;

LPDIRECTDRAW is just a typedef for IDirectDraw * . It is preferred to use ISomething *, not Microsoft's LPSOMETHING pointer types, although the wine codebase isn't really consistent as you can see in the tests. Many DirectX samples and tutorials use LPSOMETHING.
> Which NULL pointer do you mean? Does the todo_wine mean that the test reports 
> problem and just continues (so that surf is NULL)?
> Well, if ddraw::CreateSurface fails it will return an error, and 'surf' will be NULL(or undefined, not sure), which will cause problems with the GetSurfaceDesc and Release calls. So in case CreateSurface fails you'll have to skip those calls, otherwise the test will segfault.

todo_wine doesn't automatically skip or abort any tests. All it does is make sure that 'make test' doesn't fail if the test fails(and it makes sure that make test fails if the test unexpectedly succeeds)

In patch 3/4 you can remove the NULL pointer handling again, if there's no reason why the surface creation would ever fail. You can also keep it in, it's a matter of taste IMO.