public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ryan Mallon <rmallon@gmail.com>
To: Joe Perches <joe@perches.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
	Jiri Kosina <trivial@kernel.org>,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH next V2] drivers: Use NULL not zero pointer assignments
Date: Sat, 28 Jan 2012 09:27:55 +1100	[thread overview]
Message-ID: <4F2324EB.1030109@gmail.com> (raw)
In-Reply-To: <1327649890.12089.20.camel@joe2Laptop>

On 27/01/12 18:38, Joe Perches wrote:
> On Fri, 2012-01-27 at 17:20 +1100, Ryan Mallon wrote:
>> On 27/01/12 16:33, Joe Perches wrote:
>>> Using NULL pointer assignments is more kernel-style standard.
>>> Uncompiled, untested.
>>> Done via cocinelle script:
>>> @@
>>> type T;
>>> T *pointer;
>>> @@
>>> -pointer = 0
>>> +pointer = NULL
>> Hi Joe,
> Hi Ryan.
>
>> I think you can drop a lot of the initialisations completely rather than
>> convert them. I've pointed some out below. I'm just scanning through for
>> likely candidates and I didn't bother looking at the staging drivers, so
>> this list is not comprehensive.
> I'll try a more generic solution.
> Here's a possible cocci script that looks for declarations with
> initializations that are later overwritten.

I think you need to be careful with this. I don't know enough about Coccinelle, but in general control flow makes this a difficult problem. This are also the cases where a variable is assigned on all control paths before being used, but also has an (seemingly useless) initialiser because gcc occasionally cannot determine that the variable is always initialised (because in general it is undecidable) and so the initialisation is there to prevent a compiler warning.

There were also a couple of cases I pointed out where the assignment of zero to a pointer variable was actually masking some other bug, and so replacing the assignment with NULL, or removing the initialisation is not necessarily the correct fix.

A script might be useful for identifying potential cases, but I think it is worth looking at each one individually to determine what the correct fix is.

~Ryan

> $ cat multi_set.cocci
> @@
> type T;
> identifier x;
> expression y;
> expression z;
> @@
> -T x = y;
> +T x;
> ...
> x = z
> $
>
> This script has a defect because expression z may
> use identifier x.
>
> Anyway, if the script can be tweaked appropriately,
> it might be useful to remove these unnecessary
> initializations.
>
> cheers, Joe
>


  reply	other threads:[~2012-01-27 22:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20120126061330.GA15856@elgon.mountain>
2012-01-26  6:36 ` drivers: Use NULL not zero for pointer Joe Perches
2012-01-27  5:33   ` [PATCH next V2] drivers: Use NULL not zero pointer assignments Joe Perches
2012-01-27  6:20     ` Ryan Mallon
2012-01-27  7:38       ` Joe Perches
2012-01-27 22:27         ` Ryan Mallon [this message]
2012-01-27 22:46           ` Julia Lawall
2012-01-27 23:21             ` Ryan Mallon

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=4F2324EB.1030109@gmail.com \
    --to=rmallon@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=joe@perches.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=trivial@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox