Don't complain about a return when this function returns "&pdev->dev".
Reported-by: Julia Lawall <[email protected]>
Signed-off-by: Wen Yang <[email protected]>
Cc: Julia Lawall <[email protected]>
Cc: Gilles Muller <[email protected]>
Cc: Nicolas Palix <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: Masahiro Yamada <[email protected]>
CC: [email protected]
Cc: [email protected]
---
scripts/coccinelle/free/put_device.cocci | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/coccinelle/free/put_device.cocci b/scripts/coccinelle/free/put_device.cocci
index 7395697..c9f071b 100644
--- a/scripts/coccinelle/free/put_device.cocci
+++ b/scripts/coccinelle/free/put_device.cocci
@@ -32,6 +32,7 @@ if (id == NULL || ...) { ... return ...; }
( id
| (T2)dev_get_drvdata(&id->dev)
| (T3)platform_get_drvdata(id)
+| &id->dev
);
| return@p2 ...;
)
--
2.9.5
On Sat, 23 Mar 2019, Wen Yang wrote:
> Don't complain about a return when this function returns "&pdev->dev".
>
> Reported-by: Julia Lawall <[email protected]>
> Signed-off-by: Wen Yang <[email protected]>
> Cc: Julia Lawall <[email protected]>
> Cc: Gilles Muller <[email protected]>
> Cc: Nicolas Palix <[email protected]>
> Cc: Michal Marek <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> CC: [email protected]
> Cc: [email protected]
Acked-by: Julia Lawall <[email protected]>
> ---
> scripts/coccinelle/free/put_device.cocci | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/coccinelle/free/put_device.cocci b/scripts/coccinelle/free/put_device.cocci
> index 7395697..c9f071b 100644
> --- a/scripts/coccinelle/free/put_device.cocci
> +++ b/scripts/coccinelle/free/put_device.cocci
> @@ -32,6 +32,7 @@ if (id == NULL || ...) { ... return ...; }
> ( id
> | (T2)dev_get_drvdata(&id->dev)
> | (T3)platform_get_drvdata(id)
> +| &id->dev
> );
> | return@p2 ...;
> )
> --
> 2.9.5
>
>
> Don't complain about a return when this function returns "&pdev->dev".
Would this information qualify to add the tag “Fixes” to the commit message?
Regards,
Markus
On Sat, 23 Mar 2019, Markus Elfring wrote:
> > Don't complain about a return when this function returns "&pdev->dev".
>
> Would this information qualify to add the tag “Fixes” to the commit message?
Fixes tags relate to stable kernels, so that one can see which stable
kernels a particular patch should be propagated to. There is no need to
propagate patches on semantic patches to stable kernels. People who run
stable kernels are interested in their behavior, not the bug finding
rules that they contain.
julia
On Sat, Mar 23, 2019 at 09:06:54PM +0100, Julia Lawall wrote:
>
>
> On Sat, 23 Mar 2019, Markus Elfring wrote:
>
> > > Don't complain about a return when this function returns "&pdev->dev".
> >
> > Would this information qualify to add the tag “Fixes” to the commit message?
>
> Fixes tags relate to stable kernels, so that one can see which stable
> kernels a particular patch should be propagated to. There is no need to
> propagate patches on semantic patches to stable kernels. People who run
> stable kernels are interested in their behavior, not the bug finding
> rules that they contain.
The Fixes tag is not just about stable... For example, we use them for
statistics to see how quickly bugs get fixed etc.
regards,
dan carpenter
On Tue, 26 Mar 2019, Dan Carpenter wrote:
> On Sat, Mar 23, 2019 at 09:06:54PM +0100, Julia Lawall wrote:
> >
> >
> > On Sat, 23 Mar 2019, Markus Elfring wrote:
> >
> > > > Don't complain about a return when this function returns "&pdev->dev".
> > >
> > > Would this information qualify to add the tag “Fixes” to the commit message?
> >
> > Fixes tags relate to stable kernels, so that one can see which stable
> > kernels a particular patch should be propagated to. There is no need to
> > propagate patches on semantic patches to stable kernels. People who run
> > stable kernels are interested in their behavior, not the bug finding
> > rules that they contain.
>
> The Fixes tag is not just about stable... For example, we use them for
> statistics to see how quickly bugs get fixed etc.
OK. But still do we need fixes tags for bug finding rules? Perhaps if
the previous version was really broken, and it would be really undesirable
to use it.
thanks,
julia
On Tue, Mar 26, 2019 at 10:38:43AM +0100, Julia Lawall wrote:
>
>
> On Tue, 26 Mar 2019, Dan Carpenter wrote:
>
> > On Sat, Mar 23, 2019 at 09:06:54PM +0100, Julia Lawall wrote:
> > >
> > >
> > > On Sat, 23 Mar 2019, Markus Elfring wrote:
> > >
> > > > > Don't complain about a return when this function returns "&pdev->dev".
> > > >
> > > > Would this information qualify to add the tag “Fixes” to the commit message?
> > >
> > > Fixes tags relate to stable kernels, so that one can see which stable
> > > kernels a particular patch should be propagated to. There is no need to
> > > propagate patches on semantic patches to stable kernels. People who run
> > > stable kernels are interested in their behavior, not the bug finding
> > > rules that they contain.
> >
> > The Fixes tag is not just about stable... For example, we use them for
> > statistics to see how quickly bugs get fixed etc.
>
> OK. But still do we need fixes tags for bug finding rules? Perhaps if
> the previous version was really broken, and it would be really undesirable
> to use it.
It's not worth resending a patch for that, but I probably would use the
fixes tag. It depends on your definition of "bug" really... I tell
people not to use Fixes for spelling mistakes and unused variables. But
I do use the Fixes tag for things like "an off by one in a sanity check
which doesn't affect run time because the index is always correct".
regards,
dan carpenter
On Tue, 26 Mar 2019, Dan Carpenter wrote:
> On Tue, Mar 26, 2019 at 10:38:43AM +0100, Julia Lawall wrote:
> >
> >
> > On Tue, 26 Mar 2019, Dan Carpenter wrote:
> >
> > > On Sat, Mar 23, 2019 at 09:06:54PM +0100, Julia Lawall wrote:
> > > >
> > > >
> > > > On Sat, 23 Mar 2019, Markus Elfring wrote:
> > > >
> > > > > > Don't complain about a return when this function returns "&pdev->dev".
> > > > >
> > > > > Would this information qualify to add the tag “Fixes” to the commit message?
> > > >
> > > > Fixes tags relate to stable kernels, so that one can see which stable
> > > > kernels a particular patch should be propagated to. There is no need to
> > > > propagate patches on semantic patches to stable kernels. People who run
> > > > stable kernels are interested in their behavior, not the bug finding
> > > > rules that they contain.
> > >
> > > The Fixes tag is not just about stable... For example, we use them for
> > > statistics to see how quickly bugs get fixed etc.
> >
> > OK. But still do we need fixes tags for bug finding rules? Perhaps if
> > the previous version was really broken, and it would be really undesirable
> > to use it.
>
> It's not worth resending a patch for that, but I probably would use the
> fixes tag. It depends on your definition of "bug" really... I tell
> people not to use Fixes for spelling mistakes and unused variables. But
> I do use the Fixes tag for things like "an off by one in a sanity check
> which doesn't affect run time because the index is always correct".
OK, thanks.
julia
On Tue, Mar 26, 2019 at 7:16 PM Julia Lawall <[email protected]> wrote:
>
>
>
> On Tue, 26 Mar 2019, Dan Carpenter wrote:
>
> > On Tue, Mar 26, 2019 at 10:38:43AM +0100, Julia Lawall wrote:
> > >
> > >
> > > On Tue, 26 Mar 2019, Dan Carpenter wrote:
> > >
> > > > On Sat, Mar 23, 2019 at 09:06:54PM +0100, Julia Lawall wrote:
> > > > >
> > > > >
> > > > > On Sat, 23 Mar 2019, Markus Elfring wrote:
> > > > >
> > > > > > > Don't complain about a return when this function returns "&pdev->dev".
> > > > > >
> > > > > > Would this information qualify to add the tag “Fixes” to the commit message?
> > > > >
> > > > > Fixes tags relate to stable kernels, so that one can see which stable
> > > > > kernels a particular patch should be propagated to. There is no need to
> > > > > propagate patches on semantic patches to stable kernels. People who run
> > > > > stable kernels are interested in their behavior, not the bug finding
> > > > > rules that they contain.
> > > >
> > > > The Fixes tag is not just about stable... For example, we use them for
> > > > statistics to see how quickly bugs get fixed etc.
> > >
> > > OK. But still do we need fixes tags for bug finding rules? Perhaps if
> > > the previous version was really broken, and it would be really undesirable
> > > to use it.
> >
> > It's not worth resending a patch for that, but I probably would use the
> > fixes tag. It depends on your definition of "bug" really... I tell
> > people not to use Fixes for spelling mistakes and unused variables. But
> > I do use the Fixes tag for things like "an off by one in a sanity check
> > which doesn't affect run time because the index is always correct".
>
> OK, thanks.
>
> julia
I added Fixes: tag,
and applied to linux-kbuild/fixes.
Thanks.
--
Best Regards
Masahiro Yamada
On Thu, 28 Mar 2019, Masahiro Yamada wrote:
> On Tue, Mar 26, 2019 at 7:16 PM Julia Lawall <[email protected]> wrote:
> >
> >
> >
> > On Tue, 26 Mar 2019, Dan Carpenter wrote:
> >
> > > On Tue, Mar 26, 2019 at 10:38:43AM +0100, Julia Lawall wrote:
> > > >
> > > >
> > > > On Tue, 26 Mar 2019, Dan Carpenter wrote:
> > > >
> > > > > On Sat, Mar 23, 2019 at 09:06:54PM +0100, Julia Lawall wrote:
> > > > > >
> > > > > >
> > > > > > On Sat, 23 Mar 2019, Markus Elfring wrote:
> > > > > >
> > > > > > > > Don't complain about a return when this function returns "&pdev->dev".
> > > > > > >
> > > > > > > Would this information qualify to add the tag “Fixes” to the commit message?
> > > > > >
> > > > > > Fixes tags relate to stable kernels, so that one can see which stable
> > > > > > kernels a particular patch should be propagated to. There is no need to
> > > > > > propagate patches on semantic patches to stable kernels. People who run
> > > > > > stable kernels are interested in their behavior, not the bug finding
> > > > > > rules that they contain.
> > > > >
> > > > > The Fixes tag is not just about stable... For example, we use them for
> > > > > statistics to see how quickly bugs get fixed etc.
> > > >
> > > > OK. But still do we need fixes tags for bug finding rules? Perhaps if
> > > > the previous version was really broken, and it would be really undesirable
> > > > to use it.
> > >
> > > It's not worth resending a patch for that, but I probably would use the
> > > fixes tag. It depends on your definition of "bug" really... I tell
> > > people not to use Fixes for spelling mistakes and unused variables. But
> > > I do use the Fixes tag for things like "an off by one in a sanity check
> > > which doesn't affect run time because the index is always correct".
> >
> > OK, thanks.
> >
> > julia
>
> I added Fixes: tag,
> and applied to linux-kbuild/fixes.
Thanks!
julia
>
> Thanks.
>
> --
> Best Regards
> Masahiro Yamada
>
> I added Fixes: tag,
> and applied to linux-kbuild/fixes.
Will this extension trigger further adjustments for similar SmPL scripts?
Example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/scripts/coccinelle/free/pci_free_consistent.cocci?id=f7b167113753e95ae61383e234f8d10142782ace
Regards,
Markus
On Mon, 1 Apr 2019, Markus Elfring wrote:
> > I added Fixes: tag,
> > and applied to linux-kbuild/fixes.
>
> Will this extension trigger further adjustments for similar SmPL scripts?
>
> Example:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/scripts/coccinelle/free/pci_free_consistent.cocci?id=f7b167113753e95ae61383e234f8d10142782ace
If there are false positives caused by this or other scripts, feel free to
submit a patch.
julia
Some adjustments were discussed also for this script a while ago.
The software development attention evolved in a special way in the meantime.
See also for further background information:
https://lore.kernel.org/lkml/CAK7LNATjAsiSeZoTZ57zBHse0j5ZYY_12ZQ0gaF_oCziUWheOQ@mail.gmail.com/
https://systeme.lip6.fr/pipermail/cocci/2019-March/005692.html
https://lkml.org/lkml/2019/3/26/395
I would appreciate if corresponding implementation details will get
another look.
Markus Elfring (5):
Adjust a message construction
Add a cast to an expression for an assignment
Merge four SmPL when constraints into one
Extend when constraints for two SmPL ellipses
Merge two SmPL when constraints into one
scripts/coccinelle/free/put_device.cocci | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
--
2.21.0
From: Markus Elfring <[email protected]>
Date: Mon, 13 May 2019 09:47:17 +0200
A SmPL ellipsis was specified for a search approach so that additional
source code would be tolerated between an assignment to a local variable
and the corresponding null pointer check.
But such code should be restricted.
* The local variable must not be reassigned there.
* It must also not be forwarded to an other assignment target.
Take additional casts for these code exclusion specifications into account
together with optional parentheses.
Link: https://lore.kernel.org/cocci/[email protected]/
Link: https://systeme.lip6.fr/pipermail/cocci/2019-February/005620.html
Fixes: da9cfb87a44da61f2403c4312916befcb6b6d7e8 ("coccinelle: semantic code search for missing put_device()")
Signed-off-by: Markus Elfring <[email protected]>
---
scripts/coccinelle/free/put_device.cocci | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/scripts/coccinelle/free/put_device.cocci b/scripts/coccinelle/free/put_device.cocci
index aae79c02c1e0..28b0be53fb3f 100644
--- a/scripts/coccinelle/free/put_device.cocci
+++ b/scripts/coccinelle/free/put_device.cocci
@@ -13,13 +13,15 @@ virtual org
local idexpression id;
expression x,e,e1;
position p1,p2;
-type T,T1,T2,T3;
+type T,T1,T2,T3,T4,T5,T6;
@@
id = of_find_device_by_node@p1(x)
-... when != e = id
+ ... when != e = (T4)(id)
+ when != id = (T5)(e)
if (id == NULL || ...) { ... return ...; }
... when != put_device(&id->dev)
+ when != id = (T6)(e)
when != platform_device_put(id)
when != of_dev_put(id)
when != if (id) { ... put_device(&id->dev) ... }
--
2.21.0