lkml.org 
[lkml]   [2022]   [Dec]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: coccinelle: How to remove a return at the end of a void function?


On Sun, 25 Dec 2022, Uwe Kleine-König wrote:

> Hello Julia,
>
> first of all thanks for your quick answer.
>
> On Sat, Dec 24, 2022 at 01:28:04PM +0100, Julia Lawall wrote:
> > On Sat, 24 Dec 2022, Uwe Kleine-König wrote:
> > > A simplified spatch looks as follows:
> > >
> > > -------->8--------
> > > virtual patch
> > >
> > > @p1@
> > > identifier pdev;
> > > @@
> > > -int
> > > +void
> > > rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev) {
> > > <...
> > > -return 0;
> > > +return;
> > > ...>
> > > }
> > > -------->8--------
> > >
> > > This results in:
> > >
> > > -------->8--------
> > > diff -u -p a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
> > > --- a/drivers/mmc/host/rtsx_usb_sdmmc.c
> > > +++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
> > > @@ -1379,13 +1379,13 @@ static int rtsx_usb_sdmmc_drv_probe(stru
> > > return 0;
> > > }
> > >
> > > -static int rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev)
> > > +static void rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev)
> > > {
> > > struct rtsx_usb_sdmmc *host = platform_get_drvdata(pdev);
> > > struct mmc_host *mmc;
> > >
> > > if (!host)
> > > - return 0;
> > > + return;
> > >
> > > mmc = host->mmc;
> > > host->host_removal = true;
> > > @@ -1416,7 +1416,7 @@ static int rtsx_usb_sdmmc_drv_remove(str
> > > dev_dbg(&(pdev->dev),
> > > ": Realtek USB SD/MMC module has been removed\n");
> > >
> > > - return 0;
> > > + return;
> > > }
> > >
> > > #ifdef CONFIG_PM
> > > -------->8--------
> > >
> > > which is as intended. Now I want to remove the useless "return;" at the
> > > end of the function, however adding
> > >
> > > -------->8--------
> > > @p2 depends on p1@
> > > identifier pdev;
> > > @@
> > > void rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev) {
> > > ...
> > > -return;
> > > }
> > > -------->8--------
> > >
> > > to the spatch doesn't (only) do the intended:
> >
> > The problem is that Coccinelle is following the control-flow through the
> > function, and all of the returns are at the end of a control.flow path.
> > The simple, hacky solution is to change the return;s into some function
> > call Return();, then do like the above for Return(); and then change the
> > Return();s back to return;s
>
> OK, I tried, but somehow coccinelle refuse to work after I introduced
> Return(), even replacing them by return; doesn't work:
>
> -------->8--------
> virtual patch
>
> @p1@
> identifier pdev;
> @@
> -int
> +void
> rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev) {
> ...
> -return 0;
> +Return();
> ...
> }
>
> @p2@
> identifier pdev;
> @@
> void rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev) {
> ...
> -Return();
> +return;
> ...
> }

The problem is that a control-flow path at this point can have multiple
calls to Return(); You pattern only matches when every control-flow path
through the code has exactly one Return().

You should have one rule that removes the final Return();

@p2@
identifier pdev;
@@
void rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev) {
...
-Return();
}

Then another rule to remove the others:

@p2@
identifier pdev;
@@
void rtsx_usb_sdmmc_drv_remove(struct platform_device *pdev) {
<...
-Return();
+return;
...>
}

julia

> -------->8--------
>
> results in
>
> -------->8--------
> $ /usr/bin/spatch --debug -D patch --very-quiet --cocci-file scripts/coccinelle/api/test.cocci --patch . --dir drivers/mmc/host/rtsx_usb_sdmmc.c -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
> diff -u -p a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
> --- a/drivers/mmc/host/rtsx_usb_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
> @@ -1385,7 +1385,7 @@ static int rtsx_usb_sdmmc_drv_remove(str
> struct mmc_host *mmc;
>
> if (!host)
> - return 0;
> + Return();
>
> mmc = host->mmc;
> host->host_removal = true;
> @@ -1416,7 +1416,7 @@ static int rtsx_usb_sdmmc_drv_remove(str
> dev_dbg(&(pdev->dev),
> ": Realtek USB SD/MMC module has been removed\n");
>
> - return 0;
> + Return();
> }
>
> #ifdef CONFIG_PM
> -------->8--------
>
> Adding --debug doesn't give any hints.
>
> (And if I add another hunk inbeetween removing Return at the end of the
> function there is no effect either.)
>
> Do I need to split that in two spatches to make coccinelle cooperate?
>
> (If it matters, this is coccinelle as shipped by Debian, Version
> 1.1.1.deb-2)
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | https://www.pengutronix.de/ |
>
\
 
 \ /
  Last update: 2023-03-26 23:20    [W:0.063 / U:0.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site