From: Julia Lawall Julia.Lawall@lip6.fr

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 Julia.Lawall@lip6.fr

--- 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");

From: Julia Lawall Julia.Lawall@lip6.fr

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 Julia.Lawall@lip6.fr

--- 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; }

From: Julia Lawall Julia.Lawall@lip6.fr

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 Julia.Lawall@lip6.fr

--- 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; }

From: Julia Lawall Julia.Lawall@lip6.fr

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 Julia.Lawall@lip6.fr

--- 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__,

Oops!!! These are ancient patches. I meant to send something else. Sorry for the noise.

julia

On Sun, 23 Nov 2014, Julia Lawall wrote:

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>

-- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html