Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1201692 > unrolled thread

[PATCH] coccinelle: memdup.cocci: fix matching rules

Back to article view | Back to linux.kernel

Contents

  [PATCH] coccinelle: memdup.cocci: fix matching rules Andrzej Hajda <a.hajda@samsung.com> - 2015-08-06 14:20 +0200
    Re: [Cocci] [PATCH] coccinelle: memdup.cocci: fix matching rules Nicholas Mc Guire <der.herr@hofr.at> - 2015-08-06 15:40 +0200
      Re: [Cocci] [PATCH] coccinelle: memdup.cocci: fix matching rules Julia Lawall <julia.lawall@lip6.fr> - 2015-08-06 16:00 +0200
    Re: [PATCH] coccinelle: memdup.cocci: fix matching rules Julia Lawall <julia.lawall@lip6.fr> - 2015-08-06 16:40 +0200

#1201692 — [PATCH] coccinelle: memdup.cocci: fix matching rules

This patch fixes three things, listed in order of importance.
1. Removes matching of kmemdup from !patch rule - it is incorrect and
in fact makes report mode unusable.
2. Adds unlikely to if clause. It allows to match more cases - the ones with
unlikely and the ones without it.
3. Fixes report message.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi Julia,

I guess 1st and 3rd changes are OK. I am not sure about 2nd change, without
it I was not able to catch cases containing unlikely macro. For example
fs/ntfs/dir.c:1175:
  ir = kmalloc(rc, GFP_NOFS);
  if (unlikely(!ir)) {
    err = -ENOMEM;
    goto err_out;
  }
  /* Copy the index root value (it has been verified in read_inode). */
  memcpy(ir, (u8*)ctx->attr +
    le16_to_cpu(ctx->attr->data.resident.value_offset), rc);

It seems quite strange for me, as these rules looks to me isomorphic.
Is this expected behavior of coccinelle or just some bug?

After this fix, cocci finds 46 places to patch, I will send patchset if this
change looks OK to you.

I have used:
spatch version 1.0.1 with Python support and with PCRE support
latest linux-next.

Regards
Andrzej
---
 scripts/coccinelle/api/memdup.cocci | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/coccinelle/api/memdup.cocci b/scripts/coccinelle/api/memdup.cocci
index 3d1aa71..2297205 100644
--- a/scripts/coccinelle/api/memdup.cocci
+++ b/scripts/coccinelle/api/memdup.cocci
@@ -39,7 +39,7 @@ statement S;
 
 -  to = \(kmalloc@p\|kzalloc@p\)(size,flag);
 +  to = kmemdup(from,size,flag);
-   if (to==NULL || ...) S
+   if (unlikely(to==NULL) || ...) S
 -  memcpy(to, from, size);
 
 @r depends on !patch@
@@ -49,18 +49,17 @@ statement S;
 @@
 
 *  to = \(kmalloc@p\|kzalloc@p\)(size,flag);
-   to = kmemdup(from,size,flag);
-   if (to==NULL || ...) S
+   if (unlikely(to==NULL) || ...) S
 *  memcpy(to, from, size);
 
 @script:python depends on org@
 p << r.p;
 @@
 
-coccilib.org.print_todo(p[0], "WARNING opportunity for kmemdep")
+coccilib.org.print_todo(p[0], "WARNING opportunity for kmemdup")
 
 @script:python depends on report@
 p << r.p;
 @@
 
-coccilib.report.print_report(p[0], "WARNING opportunity for kmemdep")
+coccilib.report.print_report(p[0], "WARNING opportunity for kmemdup")
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1201725 — Re: [Cocci] [PATCH] coccinelle: memdup.cocci: fix matching rules

On Thu, 06 Aug 2015, Andrzej Hajda wrote:

> This patch fixes three things, listed in order of importance.
> 1. Removes matching of kmemdup from !patch rule - it is incorrect and
> in fact makes report mode unusable.
> 2. Adds unlikely to if clause. It allows to match more cases - the ones with
> unlikely and the ones without it.
> 3. Fixes report message.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi Julia,
> 
> I guess 1st and 3rd changes are OK. I am not sure about 2nd change, without
> it I was not able to catch cases containing unlikely macro. For example
> fs/ntfs/dir.c:1175:
>   ir = kmalloc(rc, GFP_NOFS);
>   if (unlikely(!ir)) {
>     err = -ENOMEM;
>     goto err_out;
>   }
>   /* Copy the index root value (it has been verified in read_inode). */
>   memcpy(ir, (u8*)ctx->attr +
>     le16_to_cpu(ctx->attr->data.resident.value_offset), rc);
> 
> It seems quite strange for me, as these rules looks to me isomorphic.
> Is this expected behavior of coccinelle or just some bug?
> 
> After this fix, cocci finds 46 places to patch, I will send patchset if this
> change looks OK to you.
> 
> I have used:
> spatch version 1.0.1 with Python support and with PCRE support
> latest linux-next.
> 
> Regards
> Andrzej
> ---
>  scripts/coccinelle/api/memdup.cocci | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/coccinelle/api/memdup.cocci b/scripts/coccinelle/api/memdup.cocci
> index 3d1aa71..2297205 100644
> --- a/scripts/coccinelle/api/memdup.cocci
> +++ b/scripts/coccinelle/api/memdup.cocci
> @@ -39,7 +39,7 @@ statement S;
>  
>  -  to = \(kmalloc@p\|kzalloc@p\)(size,flag);
>  +  to = kmemdup(from,size,flag);
> -   if (to==NULL || ...) S
> +   if (unlikely(to==NULL) || ...) S
>  -  memcpy(to, from, size);

maybe I don't understand isomorphism declarations properly but
that should actually be covered by standard.iso:

@ unlikely @
expression E;
@@
unlikely(E) <=> likely(E) => E

??
so not clear why you would get more cases with this patch.

>  
>  @r depends on !patch@
> @@ -49,18 +49,17 @@ statement S;
>  @@
>  
>  *  to = \(kmalloc@p\|kzalloc@p\)(size,flag);
> -   to = kmemdup(from,size,flag);
> -   if (to==NULL || ...) S
> +   if (unlikely(to==NULL) || ...) S
>  *  memcpy(to, from, size);
>  
>  @script:python depends on org@
>  p << r.p;
>  @@
>  
> -coccilib.org.print_todo(p[0], "WARNING opportunity for kmemdep")
> +coccilib.org.print_todo(p[0], "WARNING opportunity for kmemdup")
>  
>  @script:python depends on report@
>  p << r.p;
>  @@
>  
> -coccilib.report.print_report(p[0], "WARNING opportunity for kmemdep")
> +coccilib.report.print_report(p[0], "WARNING opportunity for kmemdup")
> -- 
> 1.9.1
> 
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201756 — Re: [Cocci] [PATCH] coccinelle: memdup.cocci: fix matching rules

On Thu, 6 Aug 2015, Nicholas Mc Guire wrote:

> On Thu, 06 Aug 2015, Andrzej Hajda wrote:
>
> > This patch fixes three things, listed in order of importance.
> > 1. Removes matching of kmemdup from !patch rule - it is incorrect and
> > in fact makes report mode unusable.
> > 2. Adds unlikely to if clause. It allows to match more cases - the ones with
> > unlikely and the ones without it.
> > 3. Fixes report message.
> >
> > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> > ---
> > Hi Julia,
> >
> > I guess 1st and 3rd changes are OK. I am not sure about 2nd change, without
> > it I was not able to catch cases containing unlikely macro. For example
> > fs/ntfs/dir.c:1175:
> >   ir = kmalloc(rc, GFP_NOFS);
> >   if (unlikely(!ir)) {
> >     err = -ENOMEM;
> >     goto err_out;
> >   }
> >   /* Copy the index root value (it has been verified in read_inode). */
> >   memcpy(ir, (u8*)ctx->attr +
> >     le16_to_cpu(ctx->attr->data.resident.value_offset), rc);
> >
> > It seems quite strange for me, as these rules looks to me isomorphic.
> > Is this expected behavior of coccinelle or just some bug?
> >
> > After this fix, cocci finds 46 places to patch, I will send patchset if this
> > change looks OK to you.
> >
> > I have used:
> > spatch version 1.0.1 with Python support and with PCRE support
> > latest linux-next.
> >
> > Regards
> > Andrzej
> > ---
> >  scripts/coccinelle/api/memdup.cocci | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/scripts/coccinelle/api/memdup.cocci b/scripts/coccinelle/api/memdup.cocci
> > index 3d1aa71..2297205 100644
> > --- a/scripts/coccinelle/api/memdup.cocci
> > +++ b/scripts/coccinelle/api/memdup.cocci
> > @@ -39,7 +39,7 @@ statement S;
> >
> >  -  to = \(kmalloc@p\|kzalloc@p\)(size,flag);
> >  +  to = kmemdup(from,size,flag);
> > -   if (to==NULL || ...) S
> > +   if (unlikely(to==NULL) || ...) S
> >  -  memcpy(to, from, size);
>
> maybe I don't understand isomorphism declarations properly but
> that should actually be covered by standard.iso:
>
> @ unlikely @
> expression E;
> @@
> unlikely(E) <=> likely(E) => E
>
> ??
> so not clear why you would get more cases with this patch.

You have to follow the arrowheads.  There is no path from E to
unlikely(E).  So adding unlikely catches both the case where it is present
and where it is not.

julia

>
> >
> >  @r depends on !patch@
> > @@ -49,18 +49,17 @@ statement S;
> >  @@
> >
> >  *  to = \(kmalloc@p\|kzalloc@p\)(size,flag);
> > -   to = kmemdup(from,size,flag);
> > -   if (to==NULL || ...) S
> > +   if (unlikely(to==NULL) || ...) S
> >  *  memcpy(to, from, size);
> >
> >  @script:python depends on org@
> >  p << r.p;
> >  @@
> >
> > -coccilib.org.print_todo(p[0], "WARNING opportunity for kmemdep")
> > +coccilib.org.print_todo(p[0], "WARNING opportunity for kmemdup")
> >
> >  @script:python depends on report@
> >  p << r.p;
> >  @@
> >
> > -coccilib.report.print_report(p[0], "WARNING opportunity for kmemdep")
> > +coccilib.report.print_report(p[0], "WARNING opportunity for kmemdup")
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Cocci mailing list
> > Cocci@systeme.lip6.fr
> > https://systeme.lip6.fr/mailman/listinfo/cocci
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201812

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

Thanks for addressing this problem.

julia

On Thu, 6 Aug 2015, Andrzej Hajda wrote:

> This patch fixes three things, listed in order of importance.
> 1. Removes matching of kmemdup from !patch rule - it is incorrect and
> in fact makes report mode unusable.
> 2. Adds unlikely to if clause. It allows to match more cases - the ones with
> unlikely and the ones without it.
> 3. Fixes report message.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi Julia,
>
> I guess 1st and 3rd changes are OK. I am not sure about 2nd change, without
> it I was not able to catch cases containing unlikely macro. For example
> fs/ntfs/dir.c:1175:
>   ir = kmalloc(rc, GFP_NOFS);
>   if (unlikely(!ir)) {
>     err = -ENOMEM;
>     goto err_out;
>   }
>   /* Copy the index root value (it has been verified in read_inode). */
>   memcpy(ir, (u8*)ctx->attr +
>     le16_to_cpu(ctx->attr->data.resident.value_offset), rc);
>
> It seems quite strange for me, as these rules looks to me isomorphic.
> Is this expected behavior of coccinelle or just some bug?
>
> After this fix, cocci finds 46 places to patch, I will send patchset if this
> change looks OK to you.
>
> I have used:
> spatch version 1.0.1 with Python support and with PCRE support
> latest linux-next.
>
> Regards
> Andrzej
> ---
>  scripts/coccinelle/api/memdup.cocci | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/coccinelle/api/memdup.cocci b/scripts/coccinelle/api/memdup.cocci
> index 3d1aa71..2297205 100644
> --- a/scripts/coccinelle/api/memdup.cocci
> +++ b/scripts/coccinelle/api/memdup.cocci
> @@ -39,7 +39,7 @@ statement S;
>
>  -  to = \(kmalloc@p\|kzalloc@p\)(size,flag);
>  +  to = kmemdup(from,size,flag);
> -   if (to==NULL || ...) S
> +   if (unlikely(to==NULL) || ...) S
>  -  memcpy(to, from, size);
>
>  @r depends on !patch@
> @@ -49,18 +49,17 @@ statement S;
>  @@
>
>  *  to = \(kmalloc@p\|kzalloc@p\)(size,flag);
> -   to = kmemdup(from,size,flag);
> -   if (to==NULL || ...) S
> +   if (unlikely(to==NULL) || ...) S
>  *  memcpy(to, from, size);
>
>  @script:python depends on org@
>  p << r.p;
>  @@
>
> -coccilib.org.print_todo(p[0], "WARNING opportunity for kmemdep")
> +coccilib.org.print_todo(p[0], "WARNING opportunity for kmemdup")
>
>  @script:python depends on report@
>  p << r.p;
>  @@
>
> -coccilib.report.print_report(p[0], "WARNING opportunity for kmemdep")
> +coccilib.report.print_report(p[0], "WARNING opportunity for kmemdup")
> --
> 1.9.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web