[v2 2/3] d3dx9: Use magic number instead of parameter tables for parameter handles.

[Matteo Bruni]

2017-04-14 12:54 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
> On 04/13/2017 08:55 PM, Matteo Bruni wrote:
>>
>> 2017-04-11 15:58 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>>>
>>> Signed-off-by: Paul Gofman <gofmanp at gmail.com>
>>>   static struct d3dx_parameter *get_valid_parameter(struct
>>> d3dx9_base_effect *base, D3DXHANDLE parameter)
>>>   {
>>> -    struct d3dx_parameter **handle_param = (struct d3dx_parameter
>>> **)parameter;
>>> +    struct d3dx_parameter *handle_param = (struct d3dx_parameter
>>> *)parameter;
>>>
>>> -    if (handle_param >= base->param_table.table && handle_param <
>>> base->param_table.table + base->param_table.count)
>>> -        return *handle_param;
>>> +    if (handle_param && !strncmp(handle_param->magic_string,
>>> parameter_magic_string,
>>> +            sizeof(parameter_magic_string)))
>>> +        return handle_param;
>>>
>>>       return get_parameter_by_name(base, NULL, parameter);
>>>   }
>>
>> Not new and somewhat separate from the patch, although it gets "more
>> interesting" with the new handles scheme: we should probably avoid the
>> call to get_parameter_by_name() if the effect was created with the
>> D3DXFX_LARGEADDRESSAWARE flag, returning NULL instead. Worth a test I
>> think.
>>
>     I did some quick testing of that when D3DXFX_LARGEADDRESSAWARE flag set,
> what I saw was:
>
> - native d3dx crashes on attempt to get or set parameter using name as
> handle.
>
> - when referencing parameter by name in IsParameterUsed, native either crash
> or returns false.
>
>     I guess native behaviour is just to treat handle as parameter pointer
> when that flag is set, without any checks. I don't achieve a reliable crash
> on parameter get and even set in our implementation though (possibly due to
> checks made in typed set's). So I think returning NULL if the magic number
> does not match as you suggest in this case (i. e. getting a predictable sure
> crash with null pointer access) is better.

Yes, agreed. Thanks for testing that out.