On 2/19/19 02:06, Michael Stefaniuc wrote:
Signed-off-by: Michael Stefaniuc mstefani@winehq.org
parameter is checked for not NULL at the beginning of the function.
Those conditional expressions are blamed on 847f772fe1d but that just split an existing if (parameter) else construct.
Yes, but the blamed conditional expression was not redundant at the time of commit 847f772fe1d or before. Unless I am missing something obvious it became redundant a few months later after commit f335932a81b.
@@ -948,15 +948,14 @@ struct d3dx_parameter *get_parameter_by_name(struct d3dx9_base_effect *base, return NULL; }
- count = parameter ? parameter->member_count : base->parameter_count;
count = parameter->member_count;
length = strcspn( name, "[.@" );
...
case '@':
{struct d3dx_top_level_parameter *top_param= top_level_parameter_from_parameter(temp_parameter);
return NULL;
return parameter ? NULL : get_annotation_by_name(base, top_param->annotation_count,top_param->annotations, part);}
It looks a bit weird now. Previously (before commit f335932a81b) '@' presence in strcspn() and switch() served the purpose of getting top level parameter annotation. Now this code just denies '@' special character in parameter name. Should not we either deny all the special characters or instead just simplify the code and remove '@' both from strcspn() and case? We do not universally check parameter names validity now (by valid I mean the names effect compiler would allow). Nor we have tests which show what will happen if the parameter in a compiled effect has an invalid name.
Just thoughts, this should better wait for Matteo of course.