[PATCH] Coccinelle : Script to detect cast after memory allocation

[PATCH] Coccinelle : Script to detect cast after memory allocation

[Himangi Saraogi]

This script detects cases of use of cast for the value returned by
kmalloc, kzalloc, kcalloc, kmem_cache_alloc, kmem_cache_zalloc,
kmem_cache_alloc_node, kmalloc_node and kzalloc_node and removes
the cast as it is not useful. This Coccinelle script replaces
drop_kmalloc_cast.cocci as it removes the casting in more limited
cases of kmalloc, kzalloc and kcalloc.

Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
 scripts/coccinelle/api/alloc/alloc_cast.cocci      | 72 ++++++++++++++++++++++
 .../coccinelle/api/alloc/drop_kmalloc_cast.cocci   | 67 --------------------
 2 files changed, 72 insertions(+), 67 deletions(-)
 create mode 100644 scripts/coccinelle/api/alloc/alloc_cast.cocci
 delete mode 100644 scripts/coccinelle/api/alloc/drop_kmalloc_cast.cocci

diff --git a/scripts/coccinelle/api/alloc/alloc_cast.cocci b/scripts/coccinelle/api/alloc/alloc_cast.cocci
new file mode 100644
index 0000000..6c308ee
--- /dev/null
+++ b/scripts/coccinelle/api/alloc/alloc_cast.cocci
@@ -0,0 +1,72 @@
+/// Remove casting the values returned by memory allocation functions
+/// like kmalloc, kzalloc, kmem_cache_alloc, kmem_cache_zalloc etc.
+///
+//# This makes an effort to find cases of casting of values returned by
+//# kmalloc, kzalloc, kcalloc, kmem_cache_alloc, kmem_cache_zalloc,
+//# kmem_cache_alloc_node, kmalloc_node and kzalloc_node and removes
+//# the casting as it is not required. The result in the patch case may
+//#need some reformatting.
+//
+// Confidence: High
+// Copyright: 2014, Himangi Saraogi  GPLv2.
+// Comments:
+// Options: --no-includes --include-headers
+//
+
+virtual context
+virtual patch
+virtual org
+virtual report
+
+//----------------------------------------------------------
+//  For context mode
+//----------------------------------------------------------
+
+@depends on context@
+type T;
+@@
+
+* (T *)
+  \(kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|kmem_cache_zalloc\|
+   kmem_cache_alloc_node\|kmalloc_node\|kzalloc_node\)(...)
+
+//----------------------------------------------------------
+//  For patch mode
+//----------------------------------------------------------
+
+@depends on patch@
+type T;
+@@
+
+- (T *)
+  (\(kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|kmem_cache_zalloc\|
+   kmem_cache_alloc_node\|kmalloc_node\|kzalloc_node\)(...))
+
+//----------------------------------------------------------
+//  For org and report mode
+//----------------------------------------------------------
+
+@r depends on org || report@
+type T;
+position p;
+@@
+
+ (T@p *)\(kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|kmem_cache_zalloc\|
+   kmem_cache_alloc_node\|kmalloc_node\|kzalloc_node\)(...)
+
+@script:python depends on org@
+p << r.p;
+t << r.T;
+@@
+
+coccilib.org.print_safe_todo(p[0], t)
+
+@script:python depends on report@
+p << r.p;
+t << r.T;
+@@
+
+msg="WARNING: casting value returned by memory allocation function to (%s *) is useless." % (t)
+coccilib.report.print_report(p[0], msg)
+
+
diff --git a/scripts/coccinelle/api/alloc/drop_kmalloc_cast.cocci b/scripts/coccinelle/api/alloc/drop_kmalloc_cast.cocci
deleted file mode 100644
index bd5d08b..0000000
--- a/scripts/coccinelle/api/alloc/drop_kmalloc_cast.cocci
+++ /dev/null
@@ -1,67 +0,0 @@
-///
-/// Casting (void *) value returned by kmalloc is useless
-/// as mentioned in Documentation/CodingStyle, Chap 14.
-///
-// Confidence: High
-// Copyright: 2009,2010 Nicolas Palix, DIKU.  GPLv2.
-// URL: http://coccinelle.lip6.fr/
-// Options: --no-includes --include-headers
-//
-// Keywords: kmalloc, kzalloc, kcalloc
-// Version min: < 2.6.12 kmalloc
-// Version min: < 2.6.12 kcalloc
-// Version min:   2.6.14 kzalloc
-//
-
-virtual context
-virtual patch
-virtual org
-virtual report
-
-//----------------------------------------------------------
-//  For context mode
-//----------------------------------------------------------
-
-@depends on context@
-type T;
-@@
-
-* (T *)
-  \(kmalloc\|kzalloc\|kcalloc\)(...)
-
-//----------------------------------------------------------
-//  For patch mode
-//----------------------------------------------------------
-
-@depends on patch@
-type T;
-@@
-
-- (T *)
-  \(kmalloc\|kzalloc\|kcalloc\)(...)
-
-//----------------------------------------------------------
-//  For org and report mode
-//----------------------------------------------------------
-
-@r depends on org || report@
-type T;
-position p;
-@@
-
- (T@p *)\(kmalloc\|kzalloc\|kcalloc\)(...)
-
-@script:python depends on org@
-p << r.p;
-t << r.T;
-@@
-
-coccilib.org.print_safe_todo(p[0], t)
-
-@script:python depends on report@
-p << r.p;
-t << r.T;
-@@
-
-msg="WARNING: casting value returned by k[cmz]alloc to (%s *) is useless." % (t)
-coccilib.report.print_report(p[0], msg)
-- 
1.9.1

Re: [PATCH] Coccinelle : Script to detect cast after memory allocation

[Joe Perches]

On Sat, 2014-06-14 at 21:15 +0530, Himangi Saraogi wrote:
> This script detects cases of use of cast for the value returned by
> kmalloc, kzalloc, kcalloc, kmem_cache_alloc, kmem_cache_zalloc,
> kmem_cache_alloc_node, kmalloc_node and kzalloc_node and removes
> the cast as it is not useful. This Coccinelle script replaces
> drop_kmalloc_cast.cocci as it removes the casting in more limited
> cases of kmalloc, kzalloc and kcalloc.

Perhaps make this more generic for any void *?

Something like:

@@
void *t;
type other;
@@

-       (other *)t
+       t

Re: [PATCH] Coccinelle : Script to detect cast after memory allocation

[Julia Lawall]

On Sat, 14 Jun 2014, Joe Perches wrote:

> On Sat, 2014-06-14 at 21:15 +0530, Himangi Saraogi wrote:
> > This script detects cases of use of cast for the value returned by
> > kmalloc, kzalloc, kcalloc, kmem_cache_alloc, kmem_cache_zalloc,
> > kmem_cache_alloc_node, kmalloc_node and kzalloc_node and removes
> > the cast as it is not useful. This Coccinelle script replaces
> > drop_kmalloc_cast.cocci as it removes the casting in more limited
> > cases of kmalloc, kzalloc and kcalloc.
> 
> Perhaps make this more generic for any void *?
> 
> Something like:
> 
> @@
> void *t;
> type other;
> @@
> 
> -       (other *)t
> +       t

Himangi, could you try it and check that you get at least the results that 
you get with the current rule?  Because I'm not sure that Coccinelle would 
have the type information available to know what is the return type of eg
kmalloc.  Maybe you would need to give the option --recursive-includes.

Perhaps it would be reasonable to add this among the specific functions.  
That is, t could be part of the disjunctions.

julia

Re: [PATCH] Coccinelle : Script to detect cast after memory allocation

[Joe Perches]

On Sat, 2014-06-14 at 18:11 +0200, Julia Lawall wrote:
> On Sat, 14 Jun 2014, Joe Perches wrote:
[]
> > Perhaps make this more generic for any void *?
> > 
> > Something like:
> > 
> > @@
> > void *t;
> > type other;
> > @@
> > 
> > -       (other *)t
> > +       t
[]
> Perhaps it would be reasonable to add this among the specific functions.  
> That is, t could be part of the disjunctions.

You do have to make sure that the
"casted to" type is not dereferenced.

ie: don't transform

void func(void *foo)
{
	unsigned long bar = *(unsigned long *)foo;
}

Also there may be some __user cast types and
such that may be necessary to exclude too.

Re: [PATCH] Coccinelle : Script to detect cast after memory allocation

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1256 bytes --]

On Sun, 15 Jun 2014, Himangi Saraogi wrote:

> Hi,
> 
> I have run the generic rule but it does not detect the cases of
> cast where the k[mzc]alloc or the kmem functions are used. I have used
> flags like recursive-includes, as suggested by Julia, but not any of the
> cases covered by the original script are detected.

Are other things detected?  You could always expand the rule to be more 
comprehensive.

julia


> Thanks
> Himangi
> 
> 
> On 14 June 2014 22:31, Joe Perches <joe@perches.com> wrote:
>       On Sat, 2014-06-14 at 18:11 +0200, Julia Lawall wrote:
>       > On Sat, 14 Jun 2014, Joe Perches wrote:
> []
> > > Perhaps make this more generic for any void *?
> > >
> > > Something like:
> > >
> > > @@
> > > void *t;
> > > type other;
> > > @@
> > >
> > > -       (other *)t
> > > +       t
> []
> > Perhaps it would be reasonable to add this among the specific
> functions.
> > That is, t could be part of the disjunctions.
> 
> You do have to make sure that the
> "casted to" type is not dereferenced.
> 
> ie: don't transform
> 
> void func(void *foo)
> {
>         unsigned long bar = *(unsigned long *)foo;
> }
> 
> Also there may be some __user cast types and
> such that may be necessary to exclude too.
> 
> 
> 
> 
> 
> 

Re: [PATCH] Coccinelle : Script to detect cast after memory allocation

[Joe Perches]

On Sun, 2014-06-15 at 01:03 +0530, Himangi Saraogi wrote:
> Hi,

Hi Himangi.

> I have run the generic rule but it does not detect the cases of
> cast where the k[mzc]alloc or the kmem functions are used. I have used
> flags like recursive-includes, as suggested by Julia, but not any of the
> cases covered by the original script are detected.

Odd.

When I tried it I got things like:

$ cat void.cocci 
@@
void *t;
type other;
@@

-	(other *)t
+	t

$ spatch --version
spatch version 1.0.0-rc14 without Python support and with PCRE support

$ spatch --sp-file void.cocci --recursive-includes drivers/block/cciss_scsi.c
drivers/block/cciss_scsi.c
+++ /tmp/cocci-output-8427-7875f0-cciss_scsi.c
@@ -704,8 +704,7 @@ cciss_scsi_setup(ctlr_info_t *h)
 	struct cciss_scsi_adapter_data_t * shba;
 
 	ccissscsi[h->ctlr].ndevices = 0;
-	shba = (struct cciss_scsi_adapter_data_t *)
-		kmalloc(sizeof(*shba), GFP_KERNEL);	
+	shba = kmalloc(sizeof(*shba), GFP_KERNEL);	
 	if (shba == NULL)
 		return;
 	shba->scsi_host = NULL;