Re: [PATCH] ata: sata_dwc_460ex: fix error path

[Vasiliy Kulikov]

Hi Sergei,

On Wed, Feb 16, 2011 at 21:10 +0300, Sergei Shtylyov wrote:
> >@@ -727,11 +727,15 @@ static int dma_dwc_xfer_setup(struct scatterlist *sg, int num_elems,
> > static void dma_dwc_exit(struct sata_dwc_device *hsdev)
> > {
> > 	dev_dbg(host_pvt.dwc_dev, "%s:\n", __func__);
> >-	if (host_pvt.sata_dma_regs)
> >+	if (host_pvt.sata_dma_regs) {
> > 		iounmap(host_pvt.sata_dma_regs);
> >+		host_pvt.sata_dma_regs = NULL;
> >+	}
> >-	if (hsdev->irq_dma)
> >+	if (hsdev->irq_dma) {
> > 		free_irq(hsdev->irq_dma, hsdev);
> >+		hsdev->irq_dma = 0;
> >+	}
> > }
> 
>    Are those changes really necessary, i.e. is dma_dwc_exit() called
> more than once?

Because this function may be called once again before
host_pvt.sata_dma_regs gains new value in sata_dwc_probe().  This would
lead to double iounmap() and double free_irq().

> >@@ -1606,7 +1610,7 @@ static int sata_dwc_probe(struct platform_device *ofdev,
> > 	if (hsdev == NULL) {
> > 		dev_err(&ofdev->dev, "kmalloc failed for hsdev\n");
> > 		err = -ENOMEM;
> >-		goto error_out;
> >+		goto error;
> 
>    Why not just:
> 
> 		return err;

If some resource allocation is added before this line in future then
"return err" should be changed.  With goto the single cleanup is just
added to the end of the function.


Thanks,

-- 
Vasiliy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/