(try 2)[PATCH 1/4] d3dcompiler: Add initial reflection parsing.

Rico Schüller kgbricola at web.de
Tue Nov 9 11:04:43 CST 2010


Am 09.11.2010 12:01, schrieb Henri Verbeet:
> 2010/11/8 Rico Schüller<kgbricola at web.de>:
>    
>> -    object->vtbl =&d3dcompiler_shader_reflection_vtbl;
>> -    object->refcount = 1;
>> +    hr = d3dcompiler_shader_reflection_init(object, data, data_size, riid);
>> +    if (FAILED(hr))
>> +    {
>> +        WARN("Failed to initialize shader reflection\n");
>> +        IUnknown_Release((IUnknown *)object);
>> +        return hr;
>> +    }
>>      
> You can't do that. Since d3dcompiler_shader_reflection_init() is
> responsible for setting the vtbl, you can't assume it's set if
> d3dcompiler_shader_reflection_init() fails. You'll have to use
> HeapFree() here.
>
>    
I made the assumption, because setting the vtbl is the first thing 
d3dcompiler_shader_reflection_init() does.
Well, a heap free isn't always enough, because there could already be 
allocated data to the object, which then would resolve in lost memory.

So my sugguestion is to move the HeapAlloc into 
d3dcompiler_shader_reflection_init(ID3D11ShaderReflection **reflector, 
const void *data, ...), from there it is safe to assume that the vtbl is 
set before the release.
>> +    if (!IsEqualGUID(riid,&IID_ID3D11ShaderReflection))
>> +    {
>> +        WARN("Wrong rrid %s, accept only %s!\n", debugstr_guid(riid), debugstr_guid(&IID_ID3D11ShaderReflection));
>> +        return E_FAIL;
>> +    }
>>      
> This can be done in D3DReflect() before even allocating the reflection
> object. (Also, "rrid" is a typo.)
>
>
>    
Yes, that can be done.

Cheers
Rico



More information about the wine-devel mailing list