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: Fri, 14 Oct 2016 17:54:45 +0530 [thread overview]
Message-ID: <5800CE8D.5080206@oracle.com> (raw)
In-Reply-To: <66694ba2-9108-b400-e412-d9927f593e16@metafoo.de>
On Friday 14 October 2016 02:21 PM, Lars-Peter Clausen wrote:
> On 10/13/2016 07:01 PM, Vaishali Thakkar wrote:
>>
>>
>> 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]
>
> To be honest, I don't remember if it was intentional or not. But looking at
> it now, I'd say context mode should use the same pattern as the report mode.
> The way it is right now context mode certainly generates a fair amount of
> false positives.
>
> As for your patch I'd say split this into multiple patches, one patch to add
> the missing devm_ variants to the context and report mode and one patch to
> improve the matching, since these are two independent changes.
Sure. I'll send the revised version with 3 patches. One more with changing
the rule of context mode.
>
--
Vaishali
WARNING: multiple messages have this Message-ID (diff)
From: Vaishali Thakkar <vaishali.thakkar@oracle.com>
To: Lars-Peter Clausen <lars@metafoo.de>,
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
Subject: Re: [PATCH] Coccinelle: misc: Improve the script for more accurate results
Date: Fri, 14 Oct 2016 17:54:45 +0530 [thread overview]
Message-ID: <5800CE8D.5080206@oracle.com> (raw)
In-Reply-To: <66694ba2-9108-b400-e412-d9927f593e16@metafoo.de>
On Friday 14 October 2016 02:21 PM, Lars-Peter Clausen wrote:
> On 10/13/2016 07:01 PM, Vaishali Thakkar wrote:
>>
>>
>> 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]
>
> To be honest, I don't remember if it was intentional or not. But looking at
> it now, I'd say context mode should use the same pattern as the report mode.
> The way it is right now context mode certainly generates a fair amount of
> false positives.
>
> As for your patch I'd say split this into multiple patches, one patch to add
> the missing devm_ variants to the context and report mode and one patch to
> improve the matching, since these are two independent changes.
Sure. I'll send the revised version with 3 patches. One more with changing
the rule of context mode.
>
--
Vaishali
next prev parent reply other threads:[~2016-10-14 12:24 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 ` [Cocci] " Vaishali Thakkar
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 ` Vaishali Thakkar [this message]
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=5800CE8D.5080206@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.