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

Henri Verbeet hverbeet at gmail.com
Thu Jun 25 08:22:24 CDT 2015

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.

More information about the wine-devel mailing list