Re: [resend PATCH 1/4] d3dx9/tests: Add test implementation for ID3DXEffectStateManager interface.

[Matteo Bruni]

2017-04-06 1:32 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
> +struct test_state_manager_update
> +{
> +    unsigned int state_op;
> +    DWORD param1;
> +    DWORD param2;
> +};
> +
> +struct test_ID3DXEffectStateManagerImpl

Sorry for being a PITA but it would be better to avoid this kind of
naming, at a first glance it might be confused with a MS structure.
Just call it test_manager or similar.
> +static HRESULT WINAPI test_ID3DXEffectStateManagerImpl_QueryInterface(ID3DXEffectStateManager *iface,
> +        REFIID riid, void **object)
> +{
> +    if (IsEqualGUID(riid, &IID_IUnknown)
> +            || IsEqualGUID(riid, &IID_ID3DXEffectStateManager))
> +    {
> +        iface->lpVtbl->AddRef(iface);
> +        *object = iface;
> +        return S_OK;
> +    }
> +
> +    return E_NOINTERFACE;
> +}

Do you know if this is called by d3dx9 at all? Either way, adding some
ok() here would be nice.
> +        state_manager->update_record = new_alloc;
> +        state_manager->update_record_size = new_size;
> +
> +    }

Spurious blank line.

Also I'm not sure it makes a lot of sense to use new_alloc / new_size
and check that the allocation succeeded with ok() to then always
assign the new variables to the struct fields anyway. Memory
allocation should never legitimately fail there so I'd remove the
vestigial error checking entirely and let the test crash if the
allocation somehow fails.