spatch for trivial pointer comparison style?

spatch for trivial pointer comparison style?

[Joe Perches]

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

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

[Julia Lawall]

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

julia

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

[Joe Perches]

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"?

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

[Julia Lawall]

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

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

[SF Markus Elfring]

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

How do you think about to adjust checks for null pointers not only
in Linux source files but also in other applications?
Are there any more software design challenges to consider with the
definition of the preprocessor symbol "NULL"?

Regards,
Markus

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

[Julia Lawall]

On Fri, 14 Nov 2014, SF Markus Elfring wrote:

> > 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.
>
> How do you think about to adjust checks for null pointers not only
> in Linux source files but also in other applications?
> Are there any more software design challenges to consider with the
> definition of the preprocessor symbol "NULL"?

Other applications may have other preferences.

julia

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

[Joe Perches]

On Fri, 2014-11-14 at 10:18 +0100, Julia Lawall wrote:
> On Thu, 13 Nov 2014, Joe Perches wrote:
[]
> > 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.

I had thought "expression *" could be r-value and
"type t; t *p;" could be l-value.

But then I don't find (or maybe don't parse too well)
the coccinelle documentation that specifies these
type relationships.

cheers, Joe

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

[Julia Lawall]

On Fri, 14 Nov 2014, Joe Perches wrote:

> On Fri, 2014-11-14 at 10:18 +0100, Julia Lawall wrote:
> > On Thu, 13 Nov 2014, Joe Perches wrote:
> []
> > > 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.
> 
> []

No. [] and * are treated completely differently.

> >   type t means that the type
> > is known.  expression *e means that there is a * in the type.
> 
> I had thought "expression *" could be r-value and
> "type t; t *p;" could be l-value.

No, you made that one up :)  As we considered that it would be common to 
want to specify the type of an expression, we thought it would be tiresome 
to have to put eg expression int x.  So you can just say int x.

The downside is that people write

identifer x;

and then don't understand the error message, because any misspelled 
metavariable kind is considered to be a type name.

julia

> But then I don't find (or maybe don't parse too well)
> the coccinelle documentation that specifies these
> type relationships.
> 
> cheers, Joe
> 
> 
>