linux-kernel - Re: [PATCH 3/3] staging: lustre: Less function calls in class_register

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]

Date:	Thu, 26 Nov 2015 23:01:30 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	SF Markus Elfring <elfring@...rs.sourceforge.net>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	devel@...verdev.osuosl.org,
	Andreas Dilger <andreas.dilger@...el.com>,
	kernel-janitors@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>,
	Oleg Drokin <oleg.drokin@...el.com>,
	Julia Lawall <julia.lawall@...6.fr>,
	lustre-devel@...ts.lustre.org
Subject: Re: [PATCH 3/3] staging: lustre: Less function calls in
 class_register_type() after error detection

These patches are often correct in the same way a stopped clock is
correct twice a day, but I reject the motivation/approach/patch
description.  Just because there is a sanity check does not mean we
should use it (ie, do an insane thing).  It hurts readability to hide
the NULL check.

On the other hand, half the time the NULL checks are superflous because
the pointer is never NULL.  The other half of the time the NULL checks
are there because the code uses one err style error handling.  Also the
error handling code should mirror the allocation code so I feel like it
should be:

	if (some_feature)
		foo = allocate();
	ret = frob();
	if (ret)
		goto free_foo;

free_foo:
	if (some_feature)  // <--- as opposed to if (foo) {
		free(foo);

So anyway I often don't like the original code, but the new code is
even worse and I have never heard a good motivation for these patches
besides that they were generated using a cool tool.  It's not a good
reason.  Other maintainers are accepting these patches so you are free
to send them there.

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/