linux-kernel - Re: [PATCH] staging: pi433: pi433_if.c Fix SET

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 1 Dec 2017 17:23:43 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Nguyen Phan Quang Minh <minhnpq16@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: pi433: pi433_if.c Fix SET_CHECKED style issues

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@...il.com>
> ---
> 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