[v4 1/4] d3dx9: Remove redundant parameter size check in set_constants().

[Paul Gofman]

On 06/21/2017 10:55 PM, Matteo Bruni wrote:
> 2017-06-19 11:24 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>> Signed-off-by: Paul Gofman <gofmanp at gmail.com>
>> ---
>> v4:
>>      - added assert() for parameter size.
>> ---
>>   dlls/d3dx9_36/preshader.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c
>> index 2785ca3..76d6de9 100644
>> --- a/dlls/d3dx9_36/preshader.c
>> +++ b/dlls/d3dx9_36/preshader.c
>> @@ -1178,11 +1178,6 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const
>>                           param_offset = i + j * info.major;
>>                       else
>>                           param_offset = i * info.minor + j;
>> -                    if (param_offset * sizeof(unsigned int) >= param->bytes)
>> -                    {
>> -                        WARN("Parameter data is too short, name %s, component %u.\n", debugstr_a(param->name), i);
>> -                        break;
>> -                    }
>>                       out[offset] = data[param_offset];
>>                   }
>>               }
>> @@ -1303,6 +1298,11 @@ static HRESULT merge_const_set_entries(struct d3dx_const_tab *const_tab,
>>               memmove(&const_tab->const_set[index + 1], &const_tab->const_set[i],
>>                       sizeof(*const_tab->const_set) * (const_tab->const_set_count - i));
>>               const_tab->const_set_count -= i - index - 1;
>> +            assert(param->bytes >= table_info[first_const->table].component_size
>> +                    * first_const->element_count
>> +                    * (first_const->direct_copy ? get_reg_components(first_const->table)
>> +                    * first_const->register_count
>> +                    : first_const->param->rows * first_const->param->columns));
> I don't think an assert() is correct here. You are comparing parameter
> size, which eventually depends on the effect data, with the constant
> size, which comes from the constant table and thus again the effect
> data. In general we shouldn't assert on invalid input data.
My point here that I don't see now how 'bytes' can be less than required 
for matrix / array. 'bytes' is computed during effect parse for such 
data and not read from the effect data. So the assert condition should 
never fire here regardless of effect data unless I am misssing something.
> Also by putting the assert() in merge_const_set_entries() you're not
> checking the non-array, non-struct constants.
It is checked more strictly with the recently added assert in 
init_set_constants_param() which was intended to check value size, 
that's why I had nothing to add there.