Re: [PATCH 8/8] mfd: lm3533: Simplify function return logic

[Javier Martinez Canillas]

Hello Johan,

On 09/30/2015 11:04 PM, Johan Hovold wrote:
> On Tue, Sep 29, 2015 at 01:26:08PM +0200, Javier Martinez Canillas wrote:
>> The invoked functions already return zero on success or a negative
>> errno code so there is no need to open code the logic in the caller.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
> 
> I do not consider this an improvement in any way and suggest this patch
> is dropped.
>

Since I posted the patch I obviously disagree but I don't really mind
if the patch is dropped.

>> ---
>>
>>  drivers/mfd/lm3533-core.c | 12 ++----------
>>  1 file changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
>> index 643f3750e830..193ecee1fa7e 100644
>> --- a/drivers/mfd/lm3533-core.c
>> +++ b/drivers/mfd/lm3533-core.c
>> @@ -472,11 +472,7 @@ static int lm3533_device_setup(struct lm3533 *lm3533,
>>  	if (ret)
>>  		return ret;
>>  
>> -	ret = lm3533_set_boost_ovp(lm3533, pdata->boost_ovp);
>> -	if (ret)
>> -		return ret;
>> -
>> -	return 0;
>> +	return lm3533_set_boost_ovp(lm3533, pdata->boost_ovp);
> 
> You're saving a few lines of code but instead introduce asymmetries and
> obscure the fact that the function returns zero on success.
>

I don't think the change makes the code more obscure tbh, the return foo()
construct is very common in the kernel and most functions return 0 on
success and a negative errno code on failure.

Also, we have a coccinelle semantic patch to find this pattern [0] so if
you think that is not worth it, please add a comment to the code. Otherwise
another developer could attempt to post the same patch since make coccicheck
will always complain about this file.

> Johan
> 

[0]: http://lxr.free-electrons.com/source/scripts/coccinelle/misc/simple_return.cocci

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/