2012-08-19 07:03:09

by Julia Lawall

Subject: [PATCH 0/10] fix error return code

These patches fix cases where the return code appears to be unintentially
nonnegative.

The complete semantic match that finds the problem is as follows:
(http://coccinelle.lip6.fr/)

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

f(...) {
<+...
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret < 0) { ... return ret; }
)
...+> }

@r exists@
identifier ret,l,ok.f;
expression e1,e2,e3;
statement S;
position p1,p2,p3;
@@

f(...) {
... when any
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
(
if (<+... ret = e3 ...+>) S
|
if (<+... &ret ...+>) S
|
if@p2(...)
{
... when != ret = e2
when forall
return@p3 ret;
}
)
... when any
}

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

(
if@p1 (\(ret < 0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
ret = e1
... when any
if@p2(...) S2

@bad2@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
@@

ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
when any
if@p2(...) S2


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

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

2012-08-19 07:03:11

by Julia Lawall

Subject: [PATCH 2/10] sound/soc/fsl/imx-sgtl5000.c: fix error return code

From: Julia Lawall <[email protected]>

Initialize ret on the second call to imx_audmux_v2_configure_port so that
the subsequent test checks that result and not the previous one.

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

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

// </smpl>

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

---
sound/soc/fsl/imx-sgtl5000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c
index fb21b17..199408e 100644
--- a/sound/soc/fsl/imx-sgtl5000.c
+++ b/sound/soc/fsl/imx-sgtl5000.c
@@ -94,7 +94,7 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "audmux internal port setup failed\n");
return ret;
}
- imx_audmux_v2_configure_port(ext_port,
+ ret = imx_audmux_v2_configure_port(ext_port,
IMX_AUDMUX_V2_PTCR_SYN,
IMX_AUDMUX_V2_PDCR_RXDSEL(int_port));
if (ret) {

2012-08-19 07:03:23

by Julia Lawall

Subject: [PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code

From: Julia Lawall <[email protected]>

Initialize ret before returning on failure, as done elsewhere in the
function.

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

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

// </smpl>

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

---
sound/ppc/snd_ps3.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 1aa52ef..9b18b52 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct ps3_system_bus_device *dev)
GFP_KERNEL);
if (!the_card.null_buffer_start_vaddr) {
pr_info("%s: nullbuffer alloc failed\n", __func__);
+ ret = -ENOMEM;
goto clean_preallocate;
}
pr_debug("%s: null vaddr=%p dma=%#llx\n", __func__,

2012-08-19 07:03:22

by Julia Lawall

Subject: [PATCH 4/9] sound/soc/omap/am3517evm.c: fix error return code

From: Julia Lawall <[email protected]>

It was forgotten to initialize ret to the result of calling
snd_soc_dai_set_sysclk, unlike at the other calls in the same function.

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

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

// </smpl>

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

---
sound/soc/omap/am3517evm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/omap/am3517evm.c b/sound/soc/omap/am3517evm.c
index 009533a..df65f98 100644
--- a/sound/soc/omap/am3517evm.c
+++ b/sound/soc/omap/am3517evm.c
@@ -59,7 +59,7 @@ static int am3517evm_hw_params(struct snd_pcm_substream *substream,
return ret;
}

- snd_soc_dai_set_sysclk(cpu_dai, OMAP_MCBSP_FSR_SRC_FSX, 0,
+ ret = snd_soc_dai_set_sysclk(cpu_dai, OMAP_MCBSP_FSR_SRC_FSX, 0,
SND_SOC_CLOCK_IN);
if (ret < 0) {
printk(KERN_ERR "can't set CPU system clock OMAP_MCBSP_FSR_SRC_FSX\n");

2012-08-19 07:03:58

by Julia Lawall

Subject: [PATCH 5/9] sound/atmel/ac97c.c: fix error return code

From: Julia Lawall <[email protected]>

In the first case, the second test of whether retval is negative is
redundant. It is dropped and the previous and subsequent tests are
combined.

In the second case, add an initialization of retval on failure of ioremap.

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

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

// </smpl>

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

---
sound/atmel/ac97c.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c
index 3c8d3ba..9052aff 100644
--- a/sound/atmel/ac97c.c
+++ b/sound/atmel/ac97c.c
@@ -278,14 +278,9 @@ static int atmel_ac97c_capture_hw_params(struct snd_pcm_substream *substream,
if (retval < 0)
return retval;
/* snd_pcm_lib_malloc_pages returns 1 if buffer is changed. */
- if (cpu_is_at32ap7000()) {
- if (retval < 0)
- return retval;
- /* snd_pcm_lib_malloc_pages returns 1 if buffer is changed. */
- if (retval == 1)
- if (test_and_clear_bit(DMA_RX_READY, &chip->flags))
- dw_dma_cyclic_free(chip->dma.rx_chan);
- }
+ if (cpu_is_at32ap7000() && retval == 1)
+ if (test_and_clear_bit(DMA_RX_READY, &chip->flags))
+ dw_dma_cyclic_free(chip->dma.rx_chan);

/* Set restrictions to params. */
mutex_lock(&opened_mutex);
@@ -980,6 +975,7 @@ static int __devinit atmel_ac97c_probe(struct platform_device *pdev)

if (!chip->regs) {
dev_dbg(&pdev->dev, "could not remap register memory\n");
+ retval = -ENOMEM;
goto err_ioremap;
}

2012-08-19 07:03:54

by Julia Lawall

Subject: [PATCH 3/10] sound/atmel/abdac.c: fix error return code

From: Julia Lawall <[email protected]>

Initialize retval before returning from a failed call to ioremap.

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

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

// </smpl>

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

---
sound/atmel/abdac.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/sound/atmel/abdac.c b/sound/atmel/abdac.c
index 98554f4..277ebce 100644
--- a/sound/atmel/abdac.c
+++ b/sound/atmel/abdac.c
@@ -452,6 +452,7 @@ static int __devinit atmel_abdac_probe(struct platform_device *pdev)
dac->regs = ioremap(regs->start, resource_size(regs));
if (!dac->regs) {
dev_dbg(&pdev->dev, "could not remap register memory\n");
+ retval = -ENOMEM;
goto out_free_card;
}

2012-08-19 07:04:29

by Julia Lawall

Subject: [PATCH 6/9] sound/pci/ctxfi/ctatc.c: fix error return code

From: Julia Lawall <[email protected]>

Initialize err before returning on failure, as done elsewhere in the
function.

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

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

// </smpl>

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

---
sound/pci/ctxfi/ctatc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/pci/ctxfi/ctatc.c b/sound/pci/ctxfi/ctatc.c
index 58b235c..a2f997a 100644
--- a/sound/pci/ctxfi/ctatc.c
+++ b/sound/pci/ctxfi/ctatc.c
@@ -1725,8 +1725,10 @@ int __devinit ct_atc_create(struct snd_card *card, struct pci_dev *pci,
atc_connect_resources(atc);

atc->timer = ct_timer_new(atc);
- if (!atc->timer)
+ if (!atc->timer) {
+ err = -ENOMEM;
goto error1;
+ }

err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, atc, &ops);
if (err < 0)

2012-08-19 07:04:52

by Julia Lawall

Subject: [PATCH 8/9] sound/pci/rme9652/hdspm.c: fix error return code

From: Julia Lawall <[email protected]>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

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

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

// </smpl>

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

---
sound/pci/rme9652/hdspm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
index b8ac871..b12308b 100644
--- a/sound/pci/rme9652/hdspm.c
+++ b/sound/pci/rme9652/hdspm.c
@@ -6585,7 +6585,7 @@ static int __devinit snd_hdspm_create(struct snd_card *card,
snd_printk(KERN_ERR "HDSPM: "
"unable to kmalloc Mixer memory of %d Bytes\n",
(int)sizeof(struct hdspm_mixer));
- return err;
+ return -ENOMEM;
}

hdspm->port_names_in = NULL;

2012-08-19 07:04:50

by Julia Lawall

Subject: [PATCH 7/9] sound/pci/sis7019.c: fix error return code

From: Julia Lawall <[email protected]>

Initialize rc before returning on failure, as done elsewhere in the
function.

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

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

// </smpl>

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

---
sound/pci/sis7019.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
index 535efe2..51e4340 100644
--- a/sound/pci/sis7019.c
+++ b/sound/pci/sis7019.c
@@ -1377,8 +1377,9 @@ static int __devinit sis_chip_create(struct snd_card *card,
if (rc)
goto error_out_cleanup;

- if (request_irq(pci->irq, sis_interrupt, IRQF_SHARED, KBUILD_MODNAME,
- sis)) {
+ rc = request_irq(pci->irq, sis_interrupt, IRQF_SHARED, KBUILD_MODNAME,
+ sis);
+ if (rc) {
dev_err(&pci->dev, "unable to allocate irq %d\n", sis->irq);
goto error_out_cleanup;
}

2012-08-19 07:05:25

by Julia Lawall

Subject: [PATCH 1/10] sound/soc/ux500/ux500_msp_i2s.c: better use devm functions and fix error return code

From: Julia Lawall <[email protected]>

Remove unnecessary calls to devm_kfree and replace iounmap by devm_iounmap
(and use resource_size for the third argument). These changes make it
possible to remove the error-handling code at the end of
ux500_msp_i2s_init_msp, and all of the gotos become direct returns.

In the case of the second call to devm_kzalloc, the return variable ret was
not initialized. Here it is changed to a direct return of -ENOMEM.

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

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

// </smpl>

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

---
sound/soc/ux500/ux500_msp_i2s.c | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index 36be11e..1b7c2f5 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -663,7 +663,6 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
struct ux500_msp **msp_p,
struct msp_i2s_platform_data *platform_data)
{
- int ret = 0;
struct resource *res = NULL;
struct i2s_controller *i2s_cont;
struct ux500_msp *msp;
@@ -687,15 +686,14 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
if (res == NULL) {
dev_err(&pdev->dev, "%s: ERROR: Unable to get resource!\n",
__func__);
- ret = -ENOMEM;
- goto err_res;
+ return -ENOMEM;
}

- msp->registers = ioremap(res->start, (res->end - res->start + 1));
+ msp->registers = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
if (msp->registers == NULL) {
dev_err(&pdev->dev, "%s: ERROR: ioremap failed!\n", __func__);
- ret = -ENOMEM;
- goto err_res;
+ return -ENOMEM;
}

msp->msp_state = MSP_STATE_IDLE;
@@ -707,7 +705,7 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
dev_err(&pdev->dev,
"%s: ERROR: Failed to allocate I2S-controller!\n",
__func__);
- goto err_i2s_cont;
+ return -ENOMEM;
}
i2s_cont->dev.parent = &pdev->dev;
i2s_cont->data = (void *)msp;
@@ -718,14 +716,6 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
msp->i2s_cont = i2s_cont;

return 0;
-
-err_i2s_cont:
- iounmap(msp->registers);
-
-err_res:
- devm_kfree(&pdev->dev, msp);
-
- return ret;
}

void ux500_msp_i2s_cleanup_msp(struct platform_device *pdev,
@@ -734,11 +724,6 @@ void ux500_msp_i2s_cleanup_msp(struct platform_device *pdev,
dev_dbg(msp->dev, "%s: Enter (id = %d).\n", __func__, msp->id);

device_unregister(&msp->i2s_cont->dev);
- devm_kfree(&pdev->dev, msp->i2s_cont);
-
- iounmap(msp->registers);
-
- devm_kfree(&pdev->dev, msp);
}

MODULE_LICENSE("GPL v2");

2012-08-19 23:05:17

by David Dillow

Subject: Re: [PATCH 7/9] sound/pci/sis7019.c: fix error return code

On Sun, 2012-08-19 at 09:02 +0200, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Initialize rc before returning on failure, as done elsewhere in the
> function.

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

Acked-by: David Dillow <[email protected]>

2012-08-20 06:49:24

by Jarkko Nikula

Subject: Re: [PATCH 4/9] sound/soc/omap/am3517evm.c: fix error return code

On 08/19/2012 10:03 AM, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> It was forgotten to initialize ret to the result of calling
> snd_soc_dai_set_sysclk, unlike at the other calls in the same function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> (
> if@p1 (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret@p1 = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
>
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> sound/soc/omap/am3517evm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/omap/am3517evm.c b/sound/soc/omap/am3517evm.c
> index 009533a..df65f98 100644
> --- a/sound/soc/omap/am3517evm.c
> +++ b/sound/soc/omap/am3517evm.c
> @@ -59,7 +59,7 @@ static int am3517evm_hw_params(struct snd_pcm_substream *substream,
> return ret;
> }
>
> - snd_soc_dai_set_sysclk(cpu_dai, OMAP_MCBSP_FSR_SRC_FSX, 0,
> + ret = snd_soc_dai_set_sysclk(cpu_dai, OMAP_MCBSP_FSR_SRC_FSX, 0,
> SND_SOC_CLOCK_IN);
> if (ret < 0) {
> printk(KERN_ERR "can't set CPU system clock OMAP_MCBSP_FSR_SRC_FSX\n");
>

Acked-by: Jarkko Nikula <[email protected]>

2012-08-20 09:05:10

by Takashi Iwai

Subject: Re: [alsa-devel] [PATCH 0/10] fix error return code

At Sun, 19 Aug 2012 09:02:51 +0200,
Julia Lawall wrote:
>
> These patches fix cases where the return code appears to be unintentially
> nonnegative.

The patch 10/10 seems missing. Or are all 9 patches? The patch
number suddenly changes to X/9 from 4.

In anyway, the patches 3, 5, 6, 7, 8 and 9 are applied.
I leave patches 1, 2 and 4 to Mark.


thanks,

Takashi

>
> The complete semantic match that finds the problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @ok exists@
> identifier f,ret,i;
> expression e;
> constant c;
> @@
>
> f(...) {
> <+...
> (
> return -c@i;
> |
> ret = -c@i;
> ... when != ret = e
> return ret;
> |
> if (ret < 0) { ... return ret; }
> )
> ...+> }
>
> @r exists@
> identifier ret,l,ok.f;
> expression e1,e2,e3;
> statement S;
> position p1,p2,p3;
> @@
>
> f(...) {
> ... when any
> (
> if@p1 (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret@p1 = 0
> )
> ... when != ret = e1
> when != &ret
> (
> if (<+... ret = e3 ...+>) S
> |
> if (<+... &ret ...+>) S
> |
> if@p2(...)
> {
> ... when != ret = e2
> when forall
> return@p3 ret;
> }
> )
> ... when any
> }
>
> @bad exists@
> position r.p1,r.p2;
> statement S1,S2;
> identifier r.ret;
> expression e1;
> @@
>
> (
> if@p1 (\(ret < 0\|ret != 0\)) S1
> |
> ret@p1 = 0
> )
> ... when any
> ret = e1
> ... when any
> if@p2(...) S2
>
> @bad2@
> position r.p1,r.p2;
> identifier r.ret;
> expression e1;
> statement S2;
> @@
>
> ret@p1 = 0
> ... when != if (...) { ... ret = e1 ... return ret; }
> when any
> if@p2(...) S2
>
>
> @script:python depends on !bad && !bad2@
> p1 << r.p1;
> p2 << r.p2;
> p3 << r.p3;
> @@
>
> cocci.print_main("",p1)
> cocci.print_secs("",p2)
> cocci.print_secs("",p3)
> // </smpl>
>
> _______________________________________________
> Alsa-devel mailing list
> [email protected]
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

2012-08-20 09:16:15

by Julia Lawall

Subject: Re: [alsa-devel] [PATCH 0/10] fix error return code

On Mon, 20 Aug 2012, Takashi Iwai wrote:

> At Sun, 19 Aug 2012 09:02:51 +0200,
> Julia Lawall wrote:
>>
>> These patches fix cases where the return code appears to be unintentially
>> nonnegative.
>
> The patch 10/10 seems missing. Or are all 9 patches? The patch
> number suddenly changes to X/9 from 4.
>
> In anyway, the patches 3, 5, 6, 7, 8 and 9 are applied.
> I leave patches 1, 2 and 4 to Mark.

Sorry, I thought I fixed that, but I guess not enough. Patches 4 and 5
were the same originally. There is no patch 10.

julia

2012-08-20 19:45:03

by Mark Brown

Subject: Re: [PATCH 2/10] sound/soc/fsl/imx-sgtl5000.c: fix error return code

On Sun, Aug 19, 2012 at 09:02:52AM +0200, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Initialize ret on the second call to imx_audmux_v2_configure_port so that
> the subsequent test checks that result and not the previous one.

Applied all ASoC patches, thanks.

Attachments: