Re: [PATCH 1/3] d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromClipper (try 5).

[Henri Verbeet]

This is by no means a full review, just a couple of obvious things I noticed.

On 24 June 2015 at 17:02, Aaryaman Vasishta  wrote:
> +static HRESULT CALLBACK surface_callback(IDirectDrawSurface *surface, DDSURFACEDESC *desc, void *context)
> +{
> - IDirectDrawClipper *d3drm_clipper = NULL;

Initializing this to NULL is pointless, you initialize it before all
uses with the IDirectDrawSurface_GetClipper() call below.
> -
>    if (context)
> -
>    {

"context" should always be non-NULL here.
> -
>        hr = IDirectDrawSurface_GetClipper(surface, &d3drm_clipper);
> -
>        ok(hr == DD_OK, "Cannot get attached clipper from primary surface (hr = %x).\n", hr);
> -
>        if (SUCCEEDED(hr))

You have an ok() one line above that states "hr == DD_OK" at this
point. If the GetClipper() call failed you'd get a test failure.

Once you've found the primary you can of course return DDENUMRET_CANCEL.

I think you're making things too complicated though. I'd just call
EnumSurfaces() with "&primary" as context pointer, set it in the
callback, and then just do the rest of the tests after EnumSurfaces()
returns, instead of trying to fit those into the callback.