LinuxLists.cc - [PATCH 00/11] fix error return code

2015-08-23 00:21:50

Subject: [PATCH 00/11] fix error return code

The complate semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@ok exists@
identifier f,ret,i;
expression e;
constant c;
@@

// identify a function that returns a negative return value at least once.
f(...) {
... when any
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret < 0) { ... return ret; }
)
... when any
}

@r exists@
identifier ret,ok.f,fn;
expression e1,e2,e3,e4,e5,e6,x;
statement S,S1;
position p1,p2,p3;
@@

// identify a case where the return variable is set to a non-negative value
// and then returned in error-handling code
f(...) {
... when any
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != &ret
when any
(
if (<+... ret = e5 ...+>) S1
|
if (<+... &ret ...+>) S1
|
if@p2(<+...x = fn(...)...+>)
{
... when != ret = e6
when forall
return@p3 ret;
}
|
break;
|
x = fn(...)
... when != \(ret = e4\|ret++\|ret--\|ret+=e4\|ret-=e4\)
when != &ret
(
if (<+... ret = e3 ...+>) S
|
if (<+... &ret ...+>) S
|
if@p2(<+...\(x != 0\|x < 0\|x == NULL\|IS_ERR(x)\)...+>)
{
... when != ret = e2
when forall
return@p3 ret;
}
)
)
... when any
}

@printer depends on r@
position p;
identifier ok.f,pr;
constant char [] c;
@@

f(...) { <...pr@p(...,c,...)...> }

@bad0 exists@
identifier r.ret,ok.f,g != {ERR_PTR,IS_ERR};
position p != printer.p;
@@

f(...) { ... when any
g@p(...,ret,...)
... when any
}

@bad depends on !bad0 exists@
position r.p1,r.p2;
statement S1,S2;
identifier r.ret;
expression e1;
@@

// ignore the above if there is some path where the variable is set to
// something else
(
if@p1 (\(ret < 0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
\(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\|&ret\)
... when any
if@p2(...) S2

@bad1 depends on !bad0 && !bad exists@
position r.p2;
statement S2;
identifier r.ret;
expression e1;
constant c;
@@

ret = -c
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != &ret
when any
if@p2(...) S2

@bad2 depends on !bad0 && !bad && !bad1 exists@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
constant c;
@@

// likewise ignore it if there has been an intervening return
ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
when != if (...) { ... return -c; }
when any
if@p2(...) S2

@script:python depends on !bad0 && !bad && !bad1 && !bad2@
p1 << r.p1;
p2 << r.p2;
p3 << r.p3;
@@

cocci.print_main("",p1)
cocci.print_secs("",p2)
cocci.print_secs("",p3)
// </smpl>

2015-08-23 00:21:22

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 01/11] mmc: omap: fix error return code

2015-08-23 00:21:52

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 02/11] soc: ti: knav_qmss_queue: fix error return code

2015-08-23 00:22:05

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 03/11] net: davinci_emac: fix error return code

Propagate error code on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
drivers/net/ethernet/ti/davinci_emac.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index aeebc0a..a21c77b 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -2004,8 +2004,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
if (res_ctrl) {
priv->ctrl_base =
devm_ioremap_resource(&pdev->dev, res_ctrl);
- if (IS_ERR(priv->ctrl_base))
+ if (IS_ERR(priv->ctrl_base)) {
+ rc = PTR_ERR(priv->ctrl_base);
goto no_pdata;
+ }
} else {
priv->ctrl_base = priv->remap_addr + pdata->ctrl_mod_reg_offset;
}

2015-08-23 00:22:06

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 04/11] fbdev: ssd1307fb: fix error return code

Propagate error code on failure. Also changed %ld to %d in dev_err to use
ret variable rather than putting two calls to PTR_ERR.

A simplified version of the semantic match that finds the first problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
drivers/video/fbdev/ssd1307fb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3e153c0..1611215 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -656,8 +656,9 @@ static int ssd1307fb_probe(struct i2c_client *client,
bl = backlight_device_register(bl_name, &client->dev, par,
&ssd1307fb_bl_ops, NULL);
if (IS_ERR(bl)) {
- dev_err(&client->dev, "unable to register backlight device: %ld\n",
- PTR_ERR(bl));
+ ret = PTR_ERR(bl);
+ dev_err(&client->dev, "unable to register backlight device: %d\n",
+ ret);
goto bl_init_error;
}

2015-08-23 00:21:58

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 05/11] mlxsw: fix error return code

2015-08-23 00:21:53

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 06/11] mmc: usdhi6rol0: fix error return code

Propagate error code on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
drivers/mmc/host/usdhi6rol0.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
index 54b082b..99f894c 100644
--- a/drivers/mmc/host/usdhi6rol0.c
+++ b/drivers/mmc/host/usdhi6rol0.c
@@ -1734,8 +1734,10 @@ static int usdhi6_probe(struct platform_device *pdev)
}

host->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(host->clk))
+ if (IS_ERR(host->clk)) {
+ ret = PTR_ERR(host->clk);
goto e_free_mmc;
+ }

host->imclk = clk_get_rate(host->clk);

2015-08-23 00:21:56

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 07/11] drm: fix error return code

Return a negative error code on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
drivers/gpu/drm/gma500/psb_drv.c | 8 ++++++--
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 92e7e57..4eff0d6 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -326,12 +326,16 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
goto out_err;

dev_priv->mmu = psb_mmu_driver_init(dev, 1, 0, 0);
- if (!dev_priv->mmu)
+ if (!dev_priv->mmu) {
+ ret = -ENOMEM;
goto out_err;
+ }

dev_priv->pf_pd = psb_mmu_alloc_pd(dev_priv->mmu, 1, 0);
- if (!dev_priv->pf_pd)
+ if (!dev_priv->pf_pd) {
+ ret = -ENOMEM;
goto out_err;
+ }

ret = psb_do_init(dev);
if (ret)
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 7841970..58a00b5 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -703,6 +703,7 @@ static int omap_dmm_probe(struct platform_device *dev)
&omap_dmm->refill_pa, GFP_KERNEL);
if (!omap_dmm->refill_va) {
dev_err(&dev->dev, "could not allocate refill memory\n");
+ ret = -ENOMEM;
goto fail;
}

2015-08-23 00:22:02

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 08/11] drm/exynos: mic: fix error return code

Propagate error code on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
drivers/gpu/drm/exynos/exynos_drm_mic.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index 8994eab..ddf0b6c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -432,6 +432,7 @@ int exynos_mic_probe(struct platform_device *pdev)
"samsung,disp-syscon");
if (IS_ERR(mic->sysreg)) {
DRM_ERROR("mic: Failed to get system register.\n");
+ ret = PTR_ERR(mic->sysreg);
goto err;
}

2015-08-23 00:21:55

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 09/11] Input: sur40: fix error return code

Propagate error code on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
drivers/input/touchscreen/sur40.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 8be7b9b..3f11763 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -581,6 +581,7 @@ static int sur40_probe(struct usb_interface *interface,
sur40->alloc_ctx = vb2_dma_sg_init_ctx(sur40->dev);
if (IS_ERR(sur40->alloc_ctx)) {
dev_err(sur40->dev, "Can't allocate buffer context");
+ error = PTR_ERR(sur40->alloc_ctx);
goto err_unreg_v4l2;
}

2015-08-23 00:22:04

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 10/11] ah6: fix error return code

2015-08-23 00:21:59

by Julia Lawall

[permalink] [raw]

Subject: [PATCH 11/11] gpio: xlp: fix error return code

2015-08-23 01:13:23

by Santosh Shilimkar

[permalink] [raw]

Subject: Re: [PATCH 02/11] soc: ti: knav_qmss_queue: fix error return code

On 8/22/15 5:11 PM, Julia Lawall wrote:
> Return a negative error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier ret; expression e1,e2;
> @@
> (
> if (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
Acked-by: Santosh Shilimkar <[email protected]>

2015-08-23 18:52:31

by Jiri Pirko

[permalink] [raw]

Subject: Re: [PATCH 05/11] mlxsw: fix error return code

Sun, Aug 23, 2015 at 02:11:16AM CEST, [email protected] wrote:
>Return a negative error code on failure.
>
>A simplified version of the semantic match that finds this problem is as
>follows: (http://coccinelle.lip6.fr/)
>
>// <smpl>
>@@
>identifier ret; expression e1,e2;
>@@
>(
>if (\(ret < 0\|ret != 0\))
> { ... return ret; }
>|
>ret = 0
>)
>... when != ret = e1
> when != &ret
>*if(...)
>{
> ... when != ret = e2
> when forall
> return ret;
>}
>// </smpl>
>
>Signed-off-by: Julia Lawall <[email protected]>

Acked-by: Jiri Pirko <[email protected]>

2015-08-24 12:13:55

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH 10/11] ah6: fix error return code

On Sun, Aug 23, 2015 at 02:11:21AM +0200, Julia Lawall wrote:
> Return a negative error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier ret; expression e1,e2;
> @@
> (
> if (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>

Acked-by: Herbert Xu <[email protected]>

This problem exists in net/ipv4/ah4.c too, right?

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2015-08-24 19:30:40

by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH 09/11] Input: sur40: fix error return code

On Sun, Aug 23, 2015 at 02:11:20AM +0200, Julia Lawall wrote:
> Propagate error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier ret; expression e1,e2;
> @@
> (
> if (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>

Applied, thank you.

>
> ---
> drivers/input/touchscreen/sur40.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 8be7b9b..3f11763 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -581,6 +581,7 @@ static int sur40_probe(struct usb_interface *interface,
> sur40->alloc_ctx = vb2_dma_sg_init_ctx(sur40->dev);
> if (IS_ERR(sur40->alloc_ctx)) {
> dev_err(sur40->dev, "Can't allocate buffer context");
> + error = PTR_ERR(sur40->alloc_ctx);
> goto err_unreg_v4l2;
> }
>
>

--
Dmitry

2015-08-25 15:26:55

by Ulf Hansson

[permalink] [raw]

Subject: Re: [PATCH 01/11] mmc: omap: fix error return code

On 23 August 2015 at 02:11, Julia Lawall <[email protected]> wrote:
> Return a negative error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier ret; expression e1,e2;
> @@
> (
> if (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>

Thanks, applied for next!

Kind regards
Uffe

>
> ---
> drivers/mmc/host/omap.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
> index 70dcf07..b763b11 100644
> --- a/drivers/mmc/host/omap.c
> +++ b/drivers/mmc/host/omap.c
> @@ -1420,8 +1420,10 @@ static int mmc_omap_probe(struct platform_device *pdev)
> host->reg_shift = (mmc_omap7xx() ? 1 : 2);
>
> host->mmc_omap_wq = alloc_workqueue("mmc_omap", 0, 0);
> - if (!host->mmc_omap_wq)
> + if (!host->mmc_omap_wq) {
> + ret = -ENOMEM;
> goto err_plat_cleanup;
> + }
>
> for (i = 0; i < pdata->nr_slots; i++) {
> ret = mmc_omap_new_slot(host, i);
>

2015-08-25 15:27:01

by Ulf Hansson

[permalink] [raw]

Subject: Re: [PATCH 06/11] mmc: usdhi6rol0: fix error return code

On 23 August 2015 at 02:11, Julia Lawall <[email protected]> wrote:
> Propagate error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier ret; expression e1,e2;
> @@
> (
> if (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>

Thanks, applied for next!

Kind regards
Uffe

>
> ---
> drivers/mmc/host/usdhi6rol0.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
> index 54b082b..99f894c 100644
> --- a/drivers/mmc/host/usdhi6rol0.c
> +++ b/drivers/mmc/host/usdhi6rol0.c
> @@ -1734,8 +1734,10 @@ static int usdhi6_probe(struct platform_device *pdev)
> }
>
> host->clk = devm_clk_get(dev, NULL);
> - if (IS_ERR(host->clk))
> + if (IS_ERR(host->clk)) {
> + ret = PTR_ERR(host->clk);
> goto e_free_mmc;
> + }
>
> host->imclk = clk_get_rate(host->clk);
>
>

2015-08-25 20:39:19

by David Miller

[permalink] [raw]

Subject: Re: [PATCH 10/11] ah6: fix error return code

From: Julia Lawall <[email protected]>
Date: Sun, 23 Aug 2015 02:11:21 +0200

> Return a negative error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <[email protected]>

Applied and as noticed by Herbert Xu I fixed up the net/ipv4/ah4.c case
as well.

2015-08-25 20:39:29

by David Miller

[permalink] [raw]

Subject: Re: [PATCH 05/11] mlxsw: fix error return code

From: Julia Lawall <[email protected]>
Date: Sun, 23 Aug 2015 02:11:16 +0200

> Return a negative error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <[email protected]>

Applied.

2015-08-25 20:39:39

by David Miller

[permalink] [raw]

Subject: Re: [PATCH 03/11] net: davinci_emac: fix error return code

From: Julia Lawall <[email protected]>
Date: Sun, 23 Aug 2015 02:11:14 +0200

> Propagate error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <[email protected]>

Applied.

2015-08-26 12:50:03

by Linus Walleij

[permalink] [raw]

Subject: Re: [PATCH 11/11] gpio: xlp: fix error return code

On Sun, Aug 23, 2015 at 2:11 AM, Julia Lawall <[email protected]> wrote:

> Return a negative error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier ret; expression e1,e2;
> @@
> (
> if (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>

Patch applied.

Yours,
Linus Walleij