From: Vasiliy Kulikov <segooon@gmail.com>
To: Sergei Shtylyov <sshtylyov@mvista.com>
Cc: Jeff Garzik <jgarzik@pobox.com>,
kernel-janitors@vger.kernel.org,
Stephen Rothwell <sfr@canb.auug.org.au>,
Rupjyoti Sarmah <rsarmah@appliedmicro.com>,
Prodyut Hazarika <phazarika@appliedmicro.com>,
Grant Likely <grant.likely@secretlab.ca>,
linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ata: sata_dwc_460ex: fix error path
Date: Fri, 18 Feb 2011 20:27:42 +0300 [thread overview]
Message-ID: <20110218172742.GA5238@albatros> (raw)
In-Reply-To: <4D5C1326.2010101@ru.mvista.com>
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
next prev parent reply other threads:[~2011-02-18 17:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-12 18:57 [PATCH] ata: sata_dwc_460ex: fix error path Vasiliy Kulikov
2011-02-16 18:10 ` Sergei Shtylyov
2011-02-18 17:27 ` Vasiliy Kulikov [this message]
2011-02-21 15:31 ` Sergei Shtylyov
2011-06-27 9:21 ` Sergei Shtylyov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110218172742.GA5238@albatros \
--to=segooon@gmail.com \
--cc=grant.likely@secretlab.ca \
--cc=jgarzik@pobox.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=phazarika@appliedmicro.com \
--cc=rsarmah@appliedmicro.com \
--cc=sfr@canb.auug.org.au \
--cc=sshtylyov@mvista.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).