[v1] coccinelle: api: Don't use devm_platform_get_and_ioremap_resource with res==NULL

Message ID 20221107114702.15706-1-u.kleine-koenig@pengutronix.de
State New
Headers
Series [v1] coccinelle: api: Don't use devm_platform_get_and_ioremap_resource with res==NULL |

Commit Message

Uwe Kleine-König Nov. 7, 2022, 11:47 a.m. UTC
  devm_platform_get_and_ioremap_resource(pdev, index, NULL) is equivalent to
the shorter devm_platform_ioremap_resource(pdev, index).

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

a potential improvement is to check for invocations of
devm_platform_get_and_ioremap_resource() where the res parameter isn't
used afterwards, but my coccinelle foo isn't strong enough for that.

Best regards
Uwe

 .../api/devm_platform_ioremap_resource.cocci  | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
  

Comments

Julia Lawall Nov. 7, 2022, 12:45 p.m. UTC | #1
On Mon, 7 Nov 2022, Uwe Kleine-König wrote:

> devm_platform_get_and_ioremap_resource(pdev, index, NULL) is equivalent to
> the shorter devm_platform_ioremap_resource(pdev, index).
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> a potential improvement is to check for invocations of
> devm_platform_get_and_ioremap_resource() where the res parameter isn't
> used afterwards, but my coccinelle foo isn't strong enough for that.

... when != res

I'm not sure where you wanted to put it though.

julia

>
> Best regards
> Uwe
>
>  .../api/devm_platform_ioremap_resource.cocci  | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
>
> diff --git a/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> new file mode 100644
> index 000000000000..401610b9a17d
> --- /dev/null
> +++ b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/// Don't use devm_platform_get_and_ioremap_resource with NULL as third parameter
> +// Confidence: High
> +
> +virtual patch
> +virtual context
> +virtual org
> +virtual report
> +
> +@r1@
> +position p;
> +@@
> + devm_platform_ioremap_resource(...) {
> +	...
> +	devm_platform_get_and_ioremap_resource@p(...)
> +	...
> + }
> +
> +@depends on patch@
> +expression pdev,index;
> +position p != r1.p;
> +@@
> +
> +-  devm_platform_get_and_ioremap_resource@p(pdev, index, NULL)
> ++  devm_platform_ioremap_resource(pdev, index)
> +
> +@r2 depends on !patch exists@
> +expression pdev,index;
> +position p;
> +@@
> +
> +*  devm_platform_get_and_ioremap_resource@p(pdev, index, NULL)
> +
> +@script:python depends on org@
> +p << r2.p;
> +@@
> +
> +cocci.print_main("WARNING opportunity for devm_platform_ioremap_resource", p)
> +
> +@script:python depends on report@
> +p << r2.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING opportunity for devm_platform_ioremap_resource")
> --
> 2.38.1
>
>
  
Uwe Kleine-König Nov. 7, 2022, 8:08 p.m. UTC | #2
On Mon, Nov 07, 2022 at 08:00:33PM +0100, Markus Elfring wrote:
> 
> > devm_platform_get_and_ioremap_resource(pdev, index, NULL) is equivalent to
> > the shorter devm_platform_ioremap_resource(pdev, index).
> …
> >  create mode 100644 scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> …
> > +@depends on patch@
> > +expression pdev,index;
> > +position p != r1.p;
> 
> 
> Why do you think that different source code positions would be required for
> your transformation approach?

That is important that the implementation of
devm_platform_ioremap_resource isn't adapted.

> > +@@
> > +
> > +-  devm_platform_get_and_ioremap_resource@p(pdev, index, NULL)
> > ++  devm_platform_ioremap_resource(pdev, index)
> 
> I suggest to use the following SmPL code variant instead.
> 
> -devm_platform_get_and_ioremap_resource@p
> +devm_platform_ioremap_resource
>  (pdev, index
> - , NULL
>  )

I don't care much, but IMHO my variant is easier to read. Might be
subjective, though.
 
> > +@r2 depends on !patch exists@
> > +expression pdev,index;
> > +position p;
> > +@@
> > +
> > +*  devm_platform_get_and_ioremap_resource@p(pdev, index, NULL)
> 
> 
> I doubt that the usage of the SmPL asterisk is appropriate for the operation
> modes “org” and “report”.

I have no idea about org and report modes. When I try these I get a
python2 error message:

	uwe@taurus:~/gsrc/linux$ make coccicheck
	You have not explicitly specified the mode to use. Using default "report" mode.
	Available modes are the following: patch, report, context, org, chain
	You can specify the mode with "make coccicheck MODE=<mode>"
	Note however that some modes are not implemented by some semantic patches.

	Please check for false positives in the output before submitting a patch.
	When using "patch" mode, carefully review the patch before submitting it.

	/usr/bin/spatch -D report --no-show-diff --very-quiet --cocci-file ./scripts/coccinelle/api/alloc/alloc_cast.cocci --no-includes --include-headers --dir . -I ./arch/x86/include -I ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi -I ./arch/x86/include/generated/uapi -I ./include/uapi -I ./include/generated/uapi --include ./include/linux/compiler-version.h --include ./include/linux/kconfig.h --jobs 4 --chunksize 1
	Py.find_library: unable to find the Python library [libpython2.7m.so returned libpython2.7m.so: cannot open shared object file: No such file or directory] [/usr/bin/../lib/libpython2.7m.so returned /usr/bin/../lib/libpython2.7m.so: cannot open shared object file: No such file or directory] [libpython2.7.so returned libpython2.7.so: cannot open shared object file: No such file or directory] [/usr/bin/../lib/libpython2.7.so returned /usr/bin/../lib/libpython2.7.so: cannot open shared object file: No such file or directory]
	coccicheck failed
	make: *** [Makefile:2076: coccicheck] Error 255

After uninstalling python2 this ends in:

	Cannot find Python library
	coccicheck failed
	make: *** [Makefile:2076: coccicheck] Error 255

Didn't try to debug that any further. Is that worth a bug report against
coccinelle (which is shipped by my distribution)?

I tried to adapt the org and report modes from other patches in the same
directory. So a critical glimpse by someone more knowledgable than me is
recommended. However I don't know how to react to "I doubt ... is
appropriate", I'd need a more constructive feedback to act on.

Best regards
Uwe
  
Julia Lawall Nov. 8, 2022, 5:51 a.m. UTC | #3
> After uninstalling python2 this ends in:
> 
>	Cannot find Python library
>	coccicheck failed
>	make: *** [Makefile:2076: coccicheck] Error 255
> 
> Didn't try to debug that any further. Is that worth a bug report against
> coccinelle (which is shipped by my distribution)?
> 
> I tried to adapt the org and report modes from other patches in the same
> directory. So a critical glimpse by someone more knowledgable than me is
> recommended. However I don't know how to react to "I doubt ... is
> appropriate", I'd need a more constructive feedback to act on.

I'm not a python expert, so I'm not sure what to do about this python2 vs python3 problem.  Is there some strategy for printing that works in both of them?

julia
  
Nicolas Palix Nov. 8, 2022, 7:55 a.m. UTC | #4
Hi all,

On 08/11/2022 06:51, Julia Lawall wrote:
> 
>> After uninstalling python2 this ends in:
>>
>> 	Cannot find Python library
>> 	coccicheck failed
>> 	make: *** [Makefile:2076: coccicheck] Error 255
>>
>> Didn't try to debug that any further. Is that worth a bug report against
>> coccinelle (which is shipped by my distribution)?
>>
>> I tried to adapt the org and report modes from other patches in the same
>> directory. So a critical glimpse by someone more knowledgable than me is
>> recommended. However I don't know how to react to "I doubt ... is
>> appropriate", I'd need a more constructive feedback to act on.
> 
> I'm not a python expert, so I'm not sure what to do about this python2 vs python3 problem.  Is there some strategy for printing that works in both of them?

It sounds like a missing dependency in the package system of the 
distribution. Coccinelle has been build with Python support, but
some libraries are missing.

Which distribution is it ?
Can you install some packages that provide the two missing shared 
librairies ?

> 
> julia
  
Uwe Kleine-König Nov. 8, 2022, 9:54 a.m. UTC | #5
Hello,

On Tue, Nov 08, 2022 at 08:55:04AM +0100, Nicolas Palix wrote:
> On 08/11/2022 06:51, Julia Lawall wrote:
> > 
> > > After uninstalling python2 this ends in:
> > > 
> > > 	Cannot find Python library
> > > 	coccicheck failed
> > > 	make: *** [Makefile:2076: coccicheck] Error 255
> > > 
> > > Didn't try to debug that any further. Is that worth a bug report against
> > > coccinelle (which is shipped by my distribution)?
> > > 
> > > I tried to adapt the org and report modes from other patches in the same
> > > directory. So a critical glimpse by someone more knowledgable than me is
> > > recommended. However I don't know how to react to "I doubt ... is
> > > appropriate", I'd need a more constructive feedback to act on.
> > 
> > I'm not a python expert, so I'm not sure what to do about this python2 vs python3 problem.  Is there some strategy for printing that works in both of them?
> 
> It sounds like a missing dependency in the package system of the
> distribution. Coccinelle has been build with Python support, but
> some libraries are missing.
> 
> Which distribution is it ?
> Can you install some packages that provide the two missing shared librairies
> ?

After installing python-is-python3 (which provides a symlink
/usr/bin/python -> python3) it works. This is on Debian testing.

Best regards
Uwe
  

Patch

diff --git a/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
new file mode 100644
index 000000000000..401610b9a17d
--- /dev/null
+++ b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
@@ -0,0 +1,44 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/// Don't use devm_platform_get_and_ioremap_resource with NULL as third parameter
+// Confidence: High
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@r1@
+position p;
+@@
+ devm_platform_ioremap_resource(...) {
+	...
+	devm_platform_get_and_ioremap_resource@p(...)
+	...
+ }
+
+@depends on patch@
+expression pdev,index;
+position p != r1.p;
+@@
+
+-  devm_platform_get_and_ioremap_resource@p(pdev, index, NULL)
++  devm_platform_ioremap_resource(pdev, index)
+
+@r2 depends on !patch exists@
+expression pdev,index;
+position p;
+@@
+
+*  devm_platform_get_and_ioremap_resource@p(pdev, index, NULL)
+
+@script:python depends on org@
+p << r2.p;
+@@
+
+cocci.print_main("WARNING opportunity for devm_platform_ioremap_resource", p)
+
+@script:python depends on report@
+p << r2.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING opportunity for devm_platform_ioremap_resource")