All of lore.kernel.org
 help / color / mirror / Atom feed
From: vaishali.thakkar@oracle.com (Vaishali Thakkar)
To: cocci@systeme.lip6.fr
Subject: [Cocci] [PATCH] Coccinelle: misc: Improve the script for more accurate results
Date: Thu, 13 Oct 2016 22:31:20 +0530	[thread overview]
Message-ID: <57FFBDE0.6000705@oracle.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1610131814180.3674@hadrien>



On Thursday 13 October 2016 09:45 PM, Julia Lawall wrote:
> 
> 
> On Thu, 13 Oct 2016, Vaishali Thakkar wrote:
> 
>> Currently because of the left associativity of the operators,
>> pattern IRQF_ONESHOT | flags does not match with the pattern
>> when we have more than one flag after the disjunction. This
>> eventually results in giving false positives by the script.
>> The patch eliminates these FPs by improving the rule.
>>
>> Also, add a new rule to eliminate the false positives given by
>> the new line issue.
>>
>> Misc:
>>
>> 1. Add support for the context, org and report mode in the case
>>    of devm_request_threaded_irq
>> 2. To be consistent with other scripts, change the confidence
>>    level to 'Moderate'
> 
> I'm getting a lot more reports for context mode than for patch mode, eg
> for sound/pcmcia/vx/vxpocket.c.  Is this normal?

This seems to be because of the ... in '*request_threaded_irq at p(...)'.
Usually I think we should have same rules for the patch  and context mode.
But the original code does not do that. So, I was not sure if that was
intentional or not.
[just in case, person wants to check all cases of these functions using
context mode]

I can send a revised version if this is not intentional. I have CC'ed the
original author of the script.

> thanks,
> julia
> 
>>
>> Signed-off-by: Vaishali Thakkar <vaishali.thakkar@oracle.com>
>> ---
>>  scripts/coccinelle/misc/irqf_oneshot.cocci | 41 +++++++++++++++++++++++++-----
>>  1 file changed, 34 insertions(+), 7 deletions(-)
>>
>> diff --git a/scripts/coccinelle/misc/irqf_oneshot.cocci b/scripts/coccinelle/misc/irqf_oneshot.cocci
>> index b421150..76fd0a2 100644
>> --- a/scripts/coccinelle/misc/irqf_oneshot.cocci
>> +++ b/scripts/coccinelle/misc/irqf_oneshot.cocci
>> @@ -5,7 +5,7 @@
>>  /// So pass the IRQF_ONESHOT flag in this case.
>>  ///
>>  //
>> -// Confidence: Good
>> +// Confidence: Moderate
>>  // Comments:
>>  // Options: --no-includes
>>
>> @@ -18,13 +18,12 @@ virtual report
>>  expression dev;
>>  expression irq;
>>  expression thread_fn;
>> -expression flags;
>>  position p;
>>  @@
>>  (
>>  request_threaded_irq at p(irq, NULL, thread_fn,
>>  (
>> -flags | IRQF_ONESHOT
>> +IRQF_ONESHOT | ...
>>  |
>>  IRQF_ONESHOT
>>  )
>> @@ -32,20 +31,40 @@ IRQF_ONESHOT
>>  |
>>  devm_request_threaded_irq at p(dev, irq, NULL, thread_fn,
>>  (
>> -flags | IRQF_ONESHOT
>> +IRQF_ONESHOT | ...
>>  |
>>  IRQF_ONESHOT
>>  )
>>  , ...)
>>  )
>>
>> - at depends on patch@
>> + at r2@
>>  expression dev;
>>  expression irq;
>>  expression thread_fn;
>>  expression flags;
>> +expression ret;
>>  position p != r1.p;
>>  @@
>> +flags = IRQF_ONESHOT | ...;
>> +(
>> +ret = request_threaded_irq at p(irq, NULL, thread_fn, flags, ...);
>> +|
>> +ret = devm_request_threaded_irq at p(dev, irq, NULL, thread_fn, flags, ...);
>> +|
>> +return request_threaded_irq at p(irq, NULL, thread_fn, flags, ...);
>> +|
>> +return devm_request_threaded_irq at p(dev, irq, NULL, thread_fn, flags, ...);
>> +)
>> +
>> + at depends on patch@
>> +expression dev;
>> +expression irq;
>> +expression thread_fn;
>> +expression flags;
>> +position p != {r1.p,r2.p};
>> +@@
>> +
>>  (
>>  request_threaded_irq at p(irq, NULL, thread_fn,
>>  (
>> @@ -69,15 +88,23 @@ devm_request_threaded_irq at p(dev, irq, NULL, thread_fn,
>>  )
>>
>>  @depends on context@
>> -position p != r1.p;
>> +position p != {r1.p,r2.p};
>>  @@
>> +(
>>  *request_threaded_irq at p(...)
>> +|
>> +*devm_request_threaded_irq at p(...)
>> +)
>>
>>  @match depends on report || org@
>>  expression irq;
>> -position p != r1.p;
>> +position p != {r1.p,r2.p};
>>  @@
>> +(
>>  request_threaded_irq at p(irq, NULL, ...)
>> +|
>> +devm_request_threaded_irq at p(dev, irq, NULL, ...)
>> +)
>>
>>  @script:python depends on org@
>>  p << match.p;
>> --
>> 2.1.4
>>
>>

-- 
Vaishali

WARNING: multiple messages have this Message-ID (diff)
From: Vaishali Thakkar <vaishali.thakkar@oracle.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: mmarek@suse.com, Gilles Muller <Gilles.Muller@lip6.fr>,
	nicolas.palix@imag.fr, cocci@systeme.lip6.fr,
	linux-kernel@vger.kernel.org,
	Lars-Peter Clausen <lars@metafoo.de>
Subject: Re: [PATCH] Coccinelle: misc: Improve the script for more accurate results
Date: Thu, 13 Oct 2016 22:31:20 +0530	[thread overview]
Message-ID: <57FFBDE0.6000705@oracle.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1610131814180.3674@hadrien>



On Thursday 13 October 2016 09:45 PM, Julia Lawall wrote:
> 
> 
> On Thu, 13 Oct 2016, Vaishali Thakkar wrote:
> 
>> Currently because of the left associativity of the operators,
>> pattern IRQF_ONESHOT | flags does not match with the pattern
>> when we have more than one flag after the disjunction. This
>> eventually results in giving false positives by the script.
>> The patch eliminates these FPs by improving the rule.
>>
>> Also, add a new rule to eliminate the false positives given by
>> the new line issue.
>>
>> Misc:
>>
>> 1. Add support for the context, org and report mode in the case
>>    of devm_request_threaded_irq
>> 2. To be consistent with other scripts, change the confidence
>>    level to 'Moderate'
> 
> I'm getting a lot more reports for context mode than for patch mode, eg
> for sound/pcmcia/vx/vxpocket.c.  Is this normal?

This seems to be because of the ... in '*request_threaded_irq@p(...)'.
Usually I think we should have same rules for the patch  and context mode.
But the original code does not do that. So, I was not sure if that was
intentional or not.
[just in case, person wants to check all cases of these functions using
context mode]

I can send a revised version if this is not intentional. I have CC'ed the
original author of the script.

> thanks,
> julia
> 
>>
>> Signed-off-by: Vaishali Thakkar <vaishali.thakkar@oracle.com>
>> ---
>>  scripts/coccinelle/misc/irqf_oneshot.cocci | 41 +++++++++++++++++++++++++-----
>>  1 file changed, 34 insertions(+), 7 deletions(-)
>>
>> diff --git a/scripts/coccinelle/misc/irqf_oneshot.cocci b/scripts/coccinelle/misc/irqf_oneshot.cocci
>> index b421150..76fd0a2 100644
>> --- a/scripts/coccinelle/misc/irqf_oneshot.cocci
>> +++ b/scripts/coccinelle/misc/irqf_oneshot.cocci
>> @@ -5,7 +5,7 @@
>>  /// So pass the IRQF_ONESHOT flag in this case.
>>  ///
>>  //
>> -// Confidence: Good
>> +// Confidence: Moderate
>>  // Comments:
>>  // Options: --no-includes
>>
>> @@ -18,13 +18,12 @@ virtual report
>>  expression dev;
>>  expression irq;
>>  expression thread_fn;
>> -expression flags;
>>  position p;
>>  @@
>>  (
>>  request_threaded_irq@p(irq, NULL, thread_fn,
>>  (
>> -flags | IRQF_ONESHOT
>> +IRQF_ONESHOT | ...
>>  |
>>  IRQF_ONESHOT
>>  )
>> @@ -32,20 +31,40 @@ IRQF_ONESHOT
>>  |
>>  devm_request_threaded_irq@p(dev, irq, NULL, thread_fn,
>>  (
>> -flags | IRQF_ONESHOT
>> +IRQF_ONESHOT | ...
>>  |
>>  IRQF_ONESHOT
>>  )
>>  , ...)
>>  )
>>
>> -@depends on patch@
>> +@r2@
>>  expression dev;
>>  expression irq;
>>  expression thread_fn;
>>  expression flags;
>> +expression ret;
>>  position p != r1.p;
>>  @@
>> +flags = IRQF_ONESHOT | ...;
>> +(
>> +ret = request_threaded_irq@p(irq, NULL, thread_fn, flags, ...);
>> +|
>> +ret = devm_request_threaded_irq@p(dev, irq, NULL, thread_fn, flags, ...);
>> +|
>> +return request_threaded_irq@p(irq, NULL, thread_fn, flags, ...);
>> +|
>> +return devm_request_threaded_irq@p(dev, irq, NULL, thread_fn, flags, ...);
>> +)
>> +
>> +@depends on patch@
>> +expression dev;
>> +expression irq;
>> +expression thread_fn;
>> +expression flags;
>> +position p != {r1.p,r2.p};
>> +@@
>> +
>>  (
>>  request_threaded_irq@p(irq, NULL, thread_fn,
>>  (
>> @@ -69,15 +88,23 @@ devm_request_threaded_irq@p(dev, irq, NULL, thread_fn,
>>  )
>>
>>  @depends on context@
>> -position p != r1.p;
>> +position p != {r1.p,r2.p};
>>  @@
>> +(
>>  *request_threaded_irq@p(...)
>> +|
>> +*devm_request_threaded_irq@p(...)
>> +)
>>
>>  @match depends on report || org@
>>  expression irq;
>> -position p != r1.p;
>> +position p != {r1.p,r2.p};
>>  @@
>> +(
>>  request_threaded_irq@p(irq, NULL, ...)
>> +|
>> +devm_request_threaded_irq@p(dev, irq, NULL, ...)
>> +)
>>
>>  @script:python depends on org@
>>  p << match.p;
>> --
>> 2.1.4
>>
>>

-- 
Vaishali

  reply	other threads:[~2016-10-13 17:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-13 10:28 [Cocci] [PATCH] Coccinelle: misc: Improve the script for more accurate results Vaishali Thakkar
2016-10-13 10:28 ` Vaishali Thakkar
2016-10-13 16:15 ` [Cocci] " Julia Lawall
2016-10-13 16:15   ` Julia Lawall
2016-10-13 17:01   ` Vaishali Thakkar [this message]
2016-10-13 17:01     ` Vaishali Thakkar
2016-10-14  8:51     ` [Cocci] " Lars-Peter Clausen
2016-10-14  8:51       ` Lars-Peter Clausen
2016-10-14 12:24       ` [Cocci] " Vaishali Thakkar
2016-10-14 12:24         ` Vaishali Thakkar

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=57FFBDE0.6000705@oracle.com \
    --to=vaishali.thakkar@oracle.com \
    --cc=cocci@systeme.lip6.fr \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.