[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 <jem456.vasishta at gmail.com> 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.