linux-kernel - Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure

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: <D1AECA61.F8DE5%andreas.dilger@intel.com>
Date:	Tue, 23 Jun 2015 08:25:05 +0000
From:	"Dilger, Andreas" <andreas.dilger@...el.com>
To:	Julia Lawall <Julia.Lawall@...6.fr>,
	"Drokin, Oleg" <oleg.drokin@...el.com>
CC:	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
	"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
	"lustre-devel@...ts.lustre.org" <lustre-devel@...ts.lustre.org>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc
 failure

On 2015/06/20, 10:58 AM, "Julia Lawall" <Julia.Lawall@...6.fr> wrote:

>!x is more normal for kzalloc failure in the kernel.

While "!x" might be more normal for kzalloc(), I don't see that as an
improvement over explicitly checking against NULL, which is what kzalloc()
and other memory-allocating functions return on error.

I've found in the past that developers can introduce bugs when they treat
return values as boolean when they really aren't.  I'd prefer that the
code is kept with explicit comparisons against NULL, as it is today.
Most of the cases that are now using "!x" are from your previous patches.

Cheers, Andreas

>Signed-off-by: Julia Lawall <Julia.Lawall@...6.fr>
>
>---
> drivers/staging/lustre/lustre/fid/fid_request.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff -u -p a/drivers/staging/lustre/lustre/fid/fid_request.c
>b/drivers/staging/lustre/lustre/fid/fid_request.c
>--- a/drivers/staging/lustre/lustre/fid/fid_request.c
>+++ b/drivers/staging/lustre/lustre/fid/fid_request.c
>@@ -498,11 +498,11 @@ int client_fid_init(struct obd_device *o
> 	int rc;
> 
> 	cli->cl_seq = kzalloc(sizeof(*cli->cl_seq), GFP_NOFS);
>-	if (cli->cl_seq == NULL)
>+	if (!cli->cl_seq)
> 		return -ENOMEM;
> 
> 	prefix = kzalloc(MAX_OBD_NAME + 5, GFP_NOFS);
>-	if (prefix == NULL) {
>+	if (!prefix) {
> 		rc = -ENOMEM;
> 		goto out_free_seq;
> 	}
>
>


Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division


--
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/