From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933723AbcJMRDp (ORCPT ); Thu, 13 Oct 2016 13:03:45 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:21435 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754954AbcJMRDY (ORCPT ); Thu, 13 Oct 2016 13:03:24 -0400 Subject: Re: [PATCH] Coccinelle: misc: Improve the script for more accurate results To: Julia Lawall References: <1476354490-25635-1-git-send-email-vaishali.thakkar@oracle.com> Cc: mmarek@suse.com, Gilles Muller , nicolas.palix@imag.fr, cocci@systeme.lip6.fr, linux-kernel@vger.kernel.org, Lars-Peter Clausen From: Vaishali Thakkar Message-ID: <57FFBDE0.6000705@oracle.com> Date: Thu, 13 Oct 2016 22:31:20 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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