public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Dilger, Andreas" <andreas.dilger@intel.com>
To: Julia Lawall <Julia.Lawall@lip6.fr>,
	"Drokin, Oleg" <oleg.drokin@intel.com>
Cc: "kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"lustre-devel@lists.lustre.org" <lustre-devel@lists.lustre.org>,
	"devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 01/12] staging: lustre: fid: Use !x to check for kzalloc failure
Date: Tue, 23 Jun 2015 08:25:05 +0000	[thread overview]
Message-ID: <D1AECA61.F8DE5%andreas.dilger@intel.com> (raw)
In-Reply-To: <1434819550-3193-2-git-send-email-Julia.Lawall@lip6.fr>

On 2015/06/20, 10:58 AM, "Julia Lawall" <Julia.Lawall@lip6.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@lip6.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



  reply	other threads:[~2015-06-23  8:25 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-20 16:58 [PATCH 00/12] Use !x to check for kzalloc failure Julia Lawall
2015-06-20 16:58 ` [PATCH 01/12] staging: lustre: fid: " Julia Lawall
2015-06-23  8:25   ` Dilger, Andreas [this message]
2015-06-23  9:23     ` Dan Carpenter
2015-06-23  9:35       ` Julia Lawall
2015-06-23  9:57         ` Dan Carpenter
2015-06-23 10:51           ` Julia Lawall
2015-06-24 20:14             ` [lustre-devel] " Simmons, James A.
2015-06-23 22:03           ` Joe Perches
2015-06-23 22:11       ` Joe Perches
2015-06-28  6:52     ` LIBCFS_ALLOC Julia Lawall
2015-06-28 21:54       ` LIBCFS_ALLOC Dan Carpenter
2015-06-30 14:56         ` LIBCFS_ALLOC Simmons, James A.
2015-06-30 15:01           ` LIBCFS_ALLOC Julia Lawall
2015-07-02 22:25             ` [lustre-devel] LIBCFS_ALLOC Simmons, James A.
2015-07-03 11:52               ` Dilger, Andreas
2015-06-30 17:38           ` LIBCFS_ALLOC Dan Carpenter
2015-06-30 21:26       ` [lustre-devel] LIBCFS_ALLOC Dilger, Andreas
2015-06-20 16:59 ` [PATCH 02/12] staging: lustre: fld: Use !x to check for kzalloc failure Julia Lawall
2015-06-20 16:59 ` [PATCH 03/12] staging: lustre: lclient: " Julia Lawall
2015-06-20 16:59 ` [PATCH 04/12] staging: lustre: ldlm: " Julia Lawall
2015-06-20 16:59 ` [PATCH 05/12] staging: lustre: lmv: " Julia Lawall
2015-06-20 16:59 ` [PATCH 06/12] staging: lustre: lov: " Julia Lawall
2015-06-20 16:59 ` [PATCH 07/12] staging: lustre: mdc: " Julia Lawall
2015-06-20 16:59 ` [PATCH 08/12] staging: lustre: mgc: " Julia Lawall
2015-06-20 16:59 ` [PATCH 09/12] staging: lustre: obdclass: " Julia Lawall
2015-06-21 10:02   ` walter harms
2015-06-21 10:29     ` Julia Lawall
2015-06-20 16:59 ` [PATCH 10/12] staging: lustre: obdecho: " Julia Lawall
2015-06-20 16:59 ` [PATCH 11/12] staging: lustre: osc: " Julia Lawall
2015-06-20 16:59 ` [PATCH 12/12] staging: lustre: ptlrpc: " Julia Lawall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D1AECA61.F8DE5%andreas.dilger@intel.com \
    --to=andreas.dilger@intel.com \
    --cc=Julia.Lawall@lip6.fr \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lustre-devel@lists.lustre.org \
    --cc=oleg.drokin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox