Re: GPU-DRM-TILCDC: Less function calls in tilcdc_convert_slave_node() after error detection

SF Markus Elfring
2016-09-22T18:38:41Z

>> The of_node_put() function was called in some cases
>> by the tilcdc_convert_slave_node() function during error handling
>> even if the passed variable contained a null pointer.
>>
>> * Adjust jump targets according to the Linux coding style convention.
>>
>> * Split a condition check for resource detection failures so that
>> each pointer from these function calls will be checked immediately.
>>
>> See also background information:
>> Topic "CWE-754: Improper check for unusual or exceptional conditions"
>> Link: https://cwe.mitre.org/data/definitions/754.html
>>
>
> I don't really agree with this patch.

This kind of feedback can be fine at first glance.

> There is no harm in calling of_node_put() with NULL as an argument

The cost of additional function calls will be eventually not noticed
just because they belong to an exception handling implementation so far.

> and because of that there is no point in making the function more complex

There is inherent software complexity involved.

> and harder to maintain.

How do you think about to discuss this aspect a bit more?

I suggest to reconsider this design detail if it is really acceptable
for the safe implementation of such a software module.

* How much will it matter in general that one function call was performed
in this use case without checking its return value immediately?

* Should it usually be determined quicker if a required resource
could be acquired before trying the next allocation?

Regards,
Markus