[PATCH] d3dx9: Remove redundant "not NULL" checks (coccinellery)

[Michael Stefaniuc]

Signed-off-by: Michael Stefaniuc <mstefani(a)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.


 dlls/d3dx9_36/effect.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
index e8ae7b54c2..1abc8dc7c8 100644
--- a/dlls/d3dx9_36/effect.c
+++ b/dlls/d3dx9_36/effect.c
@@ -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, "[.@" );
     part = name + length;
 
     for (i = 0; i < count; i++)
     {
-        temp_parameter = !parameter ? &base->parameters[i].param
-                : ¶meter->members[i];
+        temp_parameter = ¶meter->members[i];
 
         if (!strcmp(temp_parameter->name, name))
         {
@@ -971,13 +970,8 @@ struct d3dx_parameter *get_parameter_by_name(struct d3dx9_base_effect *base,
                     return get_parameter_by_name(base, temp_parameter, part);
 
                 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);
-                }
                 case '[':
                     return get_parameter_element_by_name(base, temp_parameter, part);
 
-- 
2.20.1

[Paul Gofman]

On 2/19/19 02:06, Michael Stefaniuc wrote:
> Signed-off-by: Michael Stefaniuc <mstefani(a)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.

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>