Messages in this thread | | | Date | Wed, 30 Sep 2015 17:55:13 -0400 | From | Johan Hovold <> | Subject | Re: [PATCH 8/8] mfd: lm3533: Simplify function return logic |
| |
On Wed, Sep 30, 2015 at 11:41:26PM +0200, Javier Martinez Canillas wrote: > On 09/30/2015 11:04 PM, Johan Hovold wrote: > > On Tue, Sep 29, 2015 at 01:26:08PM +0200, Javier Martinez Canillas wrote:
> >> 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.
But it was perfectly obvious from just looking at the function before your change.
> 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.
Yes, I've NAKed similar so called clean up patches based on that pattern for USB-serial and would be very glad to see that semantic patch removed.
Coccinelle can be very useful to detect and fix real bugs, but this return-value exercise is just pointless at best.
Johan
| |