Re: [v2 PATCH] d3dx9: Improve argument validation in CloneEffect()

[Matteo Bruni]

2018-03-02 14:58 GMT+01:00 Nikolay Sivov :
> Signed-off-by: Nikolay Sivov
>
> v2: using separate device now, added GetDevice() test.

Thanks, I like it but I still found something to complain about.
> @@ -7299,16 +7374,8 @@ START_TEST(effect)
>      HWND wnd;
>      IDirect3D9 *d3d;
>      IDirect3DDevice9 *device;
> -
>
> D3DPRESENT_PARAMETERS d3dpp;
> -
>
> HRESULT hr;
> ULONG refcount;
> -
>
> if (!(wnd = CreateWindowA("static", "d3dx9_test", WS_OVERLAPPEDWINDOW, 0, 0,
> -
>        640, 480, NULL, NULL, NULL, NULL)))
> -
>
> {
> -
>    skip("Couldn't create application window\n");
> -
>    return;
> -
>
> }
> if (!(d3d = Direct3DCreate9(D3D_SDK_VERSION)))
> {
>     skip("Couldn't create IDirect3D9 object\n");

A few more lines of context:

DestroyWindow(wnd);
        return;
    }

where wnd is now uninitialized.

FWIW I'd simplify it further, getting rid of the d3d argument and the
related refcount check in START_TEST. If you feel particularly fancy
you could introduce a struct test_context for storing both the device
and the window, along the lines of the d3d11 tests. That's not
required by any means though.

Ah, given that you need to resend anyway, can you please add a period
at the end of the skip() messages in create_device()?