All of lore.kernel.org
 help / color / mirror / Atom feed
From: Devin Heitmueller <dheitmueller@kernellabs.com>
To: "Peter Hüwe" <PeterHuewe@gmx.de>
Cc: Julia Lawall <julia@diku.dk>,
	Mauro Carvalho Chehab <mchehab@infradead.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org,
	Steven Toth <stoth@kernellabs.com>, Tejun Heo <tj@kernel.org>,
	Dan Carpenter <error27@gmail.com>
Subject: Re: [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer
Date: Tue, 25 Jan 2011 18:05:37 -0500	[thread overview]
Message-ID: <AANLkTinap-4djdUORmOnnnVFtTm4wSxMqTNVxrfg2jYw@mail.gmail.com> (raw)
In-Reply-To: <201101252354.31217.PeterHuewe@gmx.de>

On Tue, Jan 25, 2011 at 5:54 PM, Peter Hüwe <PeterHuewe@gmx.de> wrote:
> Hi Julia,
>
> thanks for your input.
> So do I understand you correctly if I say
> if(!x) is better than if(x==NULL) in any case?
>
> Or only for the kmalloc family?
>
> Do you remember the reason why !x should be preferred?
>
> In Documentation/CodingStyle ,  Chapter 7: Centralized exiting of functions
> there is a function fun with looks like this:
> int fun(int a)
> {
>    int result = 0;
>    char *buffer = kmalloc(SIZE);
>
>    if (buffer == NULL)
>        return -ENOMEM;
>
>    if (condition1) {
>        while (loop1) {
>            ...
>        }
>        result = 1;
>        goto out;
>    }
>    ...
> out:
>    kfree(buffer);
>    return result;
> }
>
>
> -->  So   if (buffer == NULL) is in the official CodingStyle - maybe we should
> add a paragraph there as well ;)
>
>
> Don't get me wrong, I just want to learn ;)

To my knowledge, the current CodingStyle doesn't enforce a particular
standard in this regard, leaving it at the discretion of the author.

Whether to do (!foo) or (foo == NULL) is one of those debates people
have similar to whether to use tabs as whitespace.  People have
differing opinions and there is no clearly "right" answer.  Personally
I strongly prefer (foo == NULL) as it makes it blindingly obvious that
it's a pointer comparison, whereas (!foo) leaves you wondering whether
it's an integer or pointer comparison.

All that said, you shouldn't submit patches which arbitrarily change
from one format to the other.  With regards to the proposed patch, you
should follow whatever style the author employed in the rest of the
file.

Cheers,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

WARNING: multiple messages have this Message-ID (diff)
From: Devin Heitmueller <dheitmueller@kernellabs.com>
To: "Peter Hüwe" <PeterHuewe@gmx.de>
Cc: Julia Lawall <julia@diku.dk>,
	Mauro Carvalho Chehab <mchehab@infradead.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org,
	Steven Toth <stoth@kernellabs.com>, Tejun Heo <tj@kernel.org>,
	Dan Carpenter <error27@gmail.com>
Subject: Re: [PATCH] video/saa7164: Fix sparse warning: Using plain integer as
Date: Tue, 25 Jan 2011 23:05:37 +0000	[thread overview]
Message-ID: <AANLkTinap-4djdUORmOnnnVFtTm4wSxMqTNVxrfg2jYw@mail.gmail.com> (raw)
In-Reply-To: <201101252354.31217.PeterHuewe@gmx.de>

On Tue, Jan 25, 2011 at 5:54 PM, Peter Hüwe <PeterHuewe@gmx.de> wrote:
> Hi Julia,
>
> thanks for your input.
> So do I understand you correctly if I say
> if(!x) is better than if(x=NULL) in any case?
>
> Or only for the kmalloc family?
>
> Do you remember the reason why !x should be preferred?
>
> In Documentation/CodingStyle ,  Chapter 7: Centralized exiting of functions
> there is a function fun with looks like this:
> int fun(int a)
> {
>    int result = 0;
>    char *buffer = kmalloc(SIZE);
>
>    if (buffer = NULL)
>        return -ENOMEM;
>
>    if (condition1) {
>        while (loop1) {
>            ...
>        }
>        result = 1;
>        goto out;
>    }
>    ...
> out:
>    kfree(buffer);
>    return result;
> }
>
>
> -->  So   if (buffer = NULL) is in the official CodingStyle - maybe we should
> add a paragraph there as well ;)
>
>
> Don't get me wrong, I just want to learn ;)

To my knowledge, the current CodingStyle doesn't enforce a particular
standard in this regard, leaving it at the discretion of the author.

Whether to do (!foo) or (foo = NULL) is one of those debates people
have similar to whether to use tabs as whitespace.  People have
differing opinions and there is no clearly "right" answer.  Personally
I strongly prefer (foo = NULL) as it makes it blindingly obvious that
it's a pointer comparison, whereas (!foo) leaves you wondering whether
it's an integer or pointer comparison.

All that said, you shouldn't submit patches which arbitrarily change
from one format to the other.  With regards to the proposed patch, you
should follow whatever style the author employed in the rest of the
file.

Cheers,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2011-01-25 23:05 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-25 20:54 [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer Peter Huewe
2011-01-25 20:54 ` Peter Huewe
2011-01-25 22:20 ` Julia Lawall
2011-01-25 22:20   ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer Julia Lawall
2011-01-25 22:54   ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer Peter Hüwe
2011-01-25 22:54     ` Peter Hüwe
2011-01-25 23:05     ` Devin Heitmueller [this message]
2011-01-25 23:05       ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer as Devin Heitmueller
2011-01-26  0:30       ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer Andy Walls
2011-01-26  0:30         ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer Andy Walls
2011-01-26  5:59     ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer Julia Lawall
2011-01-26  5:59       ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer Julia Lawall
2011-01-26  9:29     ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer Mauro Carvalho Chehab
2011-01-26  9:29       ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer Mauro Carvalho Chehab
2011-01-30 19:33       ` [PATCH v2] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer Peter Huewe
2011-01-30 19:33         ` Peter Huewe
2011-01-30 20:02         ` Devin Heitmueller
2011-01-30 20:02           ` [PATCH v2] video/saa7164: Fix sparse warning: Using plain integer Devin Heitmueller
2011-01-26  4:25   ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer Dan Carpenter
2011-01-26  4:25     ` [PATCH] video/saa7164: Fix sparse warning: Using plain integer Dan Carpenter

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=AANLkTinap-4djdUORmOnnnVFtTm4wSxMqTNVxrfg2jYw@mail.gmail.com \
    --to=dheitmueller@kernellabs.com \
    --cc=PeterHuewe@gmx.de \
    --cc=error27@gmail.com \
    --cc=julia@diku.dk \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=stoth@kernellabs.com \
    --cc=tj@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.