Re: [media] ov5645: Move an error code assignment in ov5645_probe()

From: SF Markus Elfring
Date: Thu Mar 15 2018 - 05:35:21 EST


>> Move an assignment for a specific error code so that it is stored only once
>> in this function implementation.
>>
>> This issue was detected by using the Coccinelle software.
>
> How?

Would you like to experiment a bit more with the following approach
for the semantic patch language?

show_same_statements3.cocci:

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
T work(...)
{
... when any
*if ((...) < 0)
*{
...
* s1
* s2
*}
... when any
*if ((...) < 0)
*{
...
* s1
* s2
*}
... when any
}


>> @@ -1334,6 +1329,7 @@ static int ov5645_probe(struct i2c_client *client,
>>
>> power_down:
>> ov5645_s_power(&ov5645->sd, false);
>> + ret = -ENODEV;
>
> I don't think this is where people would expect you to set the error code
> in general.

This can be. - The view depends on some factors.


> It should rather take place before goto, not after it.

I proposed another software design direction.


> That'd mean another variable,

To which detail do you refer here?


> and I'm not convinced the result would improve the driver.

Can you see the relevance of a small code reduction in this function?

Regards,
Markus