Re: [PATCH] staging: pi433: pi433_if.c Fix SET_CHECKED style issues

From: Dan Carpenter
Date: Fri Dec 01 2017 - 09:23:59 EST


On Thu, Nov 30, 2017 at 10:40:14PM +0100, Nguyen Phan Quang Minh wrote:
> Fix checkpatch warning and add result holder variable to reduce overhead
> since the macro is called on functions.
>
> Signed-off-by: Nguyen Phan Quang Minh <minhnpq16@xxxxxxxxx>
> ---
> Since SET_CHECKED has a return statement, I'm very tempted to straight
> up remove it and do the checking inline. But the macro is used a lot
> (~60 times), I think it might hurt the readability of the code.
>

Yeah. Last time I saw this I said to kill it with fire but now that
you've fixed the major bug I'm less destructive I guess...

https://lkml.org/lkml/2017/7/28/401

The other typical bug with hidden returns is that we don't unlock or
release resources on error. There are some of those bugs in the probe
function.

We definitely should get rid of the SET_CHECKED() macro, and if you do
it in a very mechanical no-thinking type of sed way, I won't complain.
But it would be nice to take some time and try to do it nicely.

regards,
dan carpenter