linux-kernel - Re: s390/net: Deletion of unnecessary checks before two function calls

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20141103162528.GT6890@mwanda>
Date:	Mon, 3 Nov 2014 19:25:28 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	SF Markus Elfring <elfring@...rs.sourceforge.net>
Cc:	Ursula Braun <ursula.braun@...ibm.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Frank Blaschka <blaschka@...ux.vnet.ibm.com>,
	linux390@...ibm.com, linux-s390@...r.kernel.org,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
	trivial@...nel.org, Coccinelle <cocci@...teme.lip6.fr>
Subject: Re: s390/net: Deletion of unnecessary checks before two function
 calls

On Mon, Nov 03, 2014 at 04:55:12PM +0100, SF Markus Elfring wrote:
> > This one is buggy.
> 
> I am still interested to clarify this opinion a bit more.
> 

After your patch then it will print warning messages.

The truth is I think that all these patches are bad and they make the
code harder to read.

Before:  The code is clear and there is no NULL dereference.
 After:  You have to remember that rtw_free_netdev() accepts NULL
	 pointers but free_netdev() does not accept NULL pointers.

The if statements are there for *human* readers to understand and you are
making it harder for humans to understand the code.

Even for kfree(), just removing the if statement is not really the right
fix.  We do it because everyone knows kfree(), but what Julia Lawall
said is the real correct way change the code and make it simpler for
people to understand:

https://lkml.org/lkml/2014/10/31/452

I know it's fun to send automated patches but these make the code worse
and they waste reviewer time.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/