linux-kernel - Re: [Cocci] spatch for trivial pointer comparison style?

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: <alpine.DEB.2.10.1411140938400.2443@hadrien>
Date:	Fri, 14 Nov 2014 10:18:41 +0100 (CET)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	Joe Perches <joe@...ches.com>
cc:	cocci <cocci@...teme.lip6.fr>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [Cocci] spatch for trivial pointer comparison style?

On Thu, 13 Nov 2014, Joe Perches wrote:

> On Fri, 2014-11-14 at 07:06 +0100, Julia Lawall wrote:
> > On Thu, 13 Nov 2014, Joe Perches wrote:
> >
> > > I added a checkpatch entry for this.
> > > Maybe some cocci test like this would be useful?
> > >
> > > @@
> > > type t;
> > > t *p;
> > > @@
> > > -	p == NULL
> > > +	!p
> > >
> > > @@
> > > type t;
> > > t *p;
> > > @@
> > > -	p != NULL
> > > +	p
> > >
> > > @@
> > > type t;
> > > t *p;
> > > @@
> > > -	NULL == p
> > > +	!p
> > >
> > > @@
> > > type t;
> > > t *p;
> > > @@
> > > -	NULL != p
> > > +	p
> >
> > This was discussed many years ago.  I don't think that the change is
> > desirable in all cases.  There are functions like kmalloc where NULL means
> > failure and !p seems like the reasonable choice.  But there maybe other
> > cases where NULL is somehow a meaningful value.
> >
> > Here is a link to the part of the discussion:
> >
> > https://lkml.org/lkml/2007/7/27/103
>
> Yes, I agree with some of the things Al Viro said
> there, but isn't 'type t; t *p;' a subset of
> "expression *e"?

No.  How would you expect it to be different.  type t means that the type
is known.  expression *e means that there is a * in the type.  But there
is no way to know that there is a * in the type without knowing the full
type.

Maybe something like

e = f(...);
...
if (e == NULL) S1 else S2

would be acceptable?  But I was thinking that for some functions NULL
might be considered to be a meaningful result, rather than a sign of
failure.

The following semantic patch gives almost 3000 results:

@disable is_null@
expression e;
statement S1,S2;
@@

e = \(kmalloc\|kzalloc\|kcalloc\|devm_kmalloc\|devm_kzalloc\)(...);
... when != e
if (<+...
- e == NULL
+ !e
    ...+>) S1 else S2

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