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

[Paul Gofman]

I am going to send this patch (along with a few other improvements we 
discussed) at some point. Just if you don't mind I would move straight 
ahead to finalize shared parameters, to make the effect thing finally 
look mostly complete.

On 04/17/2017 11:40 PM, Matteo Bruni wrote:
> 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.
>