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

[Matteo Bruni]

2018-03-01 13:14 GMT+01:00 Nikolay Sivov :
> diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
> index 1900f3eac1..28dcf47cd3 100644
> --- a/dlls/d3dx9_36/tests/effect.c
> +++ b/dlls/d3dx9_36/tests/effect.c
> @@ -7294,6 +7294,91 @@ static void test_effect_null_shader(void)
>      DestroyWindow(window);
>  }
>
> +static void test_effect_clone(IDirect3DDevice9 *device)

Could you make the test create its own device instead, like the last
two tests added to the file?
> - memset(&present_parameters, 0, sizeof(present_parameters));
> - present_parameters.Windowed = TRUE;
> - present_parameters.SwapEffect = D3DSWAPEFFECT_DISCARD;
> - hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window,
> -
>        D3DCREATE_HARDWARE_VERTEXPROCESSING, &present_parameters, &device2);
> - ok(SUCCEEDED(hr), "Failed to create IDirect3DDevice9 object, hr %#x\n", hr);
> -
> - hr = effect->lpVtbl->CloneEffect(effect, device2, &cloned);
>
> +todo_wine
> - ok(hr == D3D_OK, "Got result %#x.\n", hr);
>
> +if (hr == D3D_OK)
> +{
> - ok(cloned != effect, "Expected new effect instance.\n");
> - cloned->lpVtbl->Release(cloned);
>
> +}

Here you could also check (via the GetDevice() method) that the cloned
effect actually uses the new device. I don't expect surprises though.