netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jia-Ju Bai <baijiaju1990@163.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: davem@davemloft.net, ebiederm@xmission.com,
	dingtianhong@huawei.com, paul.gortmaker@windriver.com,
	justinvanwijngaarden@gmail.com, netdev@vger.kernel.org
Subject: Re: [PATCH v3] 3c59x: Fix memory leaks in vortex_open
Date: Tue, 23 Dec 2014 23:00:01 +0800	[thread overview]
Message-ID: <54998371.7060109@163.com> (raw)
In-Reply-To: <20141223142439.GD31876@hmsreliant.think-freely.org>

Thanks for the reply!

> This doesn't make sense.  We free all the skbs in vortex_open if we don't
> allocate all of them (the if (i != RX_RING_SIZE) check), the only place we miss
> is if vortex_up fails, and you didn't remove the if (!retval) goto out check, so
> this code won't get run appropriately.

In the code, when vortex_up is failed and does not returns 0,
"if (!retval)" is failed and "goto out" is not executed, so error 
handling code
below is executed, including my added code.
Is it right?

>
> That said, it does seem we need to clean up if vortex_up fails, but it would
> seem to me to be easier to just call vortex_close if it does, since that will do
> all of the approriate cleanup.
>
> Neil
>

In the code, vortex_close does too many releasing operations, such as free
vp->tx_skbuff, but vortex_open only allocates memory for vp->rx_skbuff 
before
vortex_up is called.
So I think it is enough to just free the memory of vp->rx_skbuff when 
vortex_up
is failed.

  reply	other threads:[~2014-12-23 15:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-23  2:54 [PATCH v3] 3c59x: Fix memory leaks in vortex_open Jia-Ju Bai
2014-12-23 14:24 ` Neil Horman
2014-12-23 15:00   ` Jia-Ju Bai [this message]
2014-12-23 15:43     ` Neil Horman
2014-12-24  2:12       ` Jia-Ju Bai
2014-12-24  3:27         ` Neil Horman
2014-12-24  4:10           ` David Miller

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=54998371.7060109@163.com \
    --to=baijiaju1990@163.com \
    --cc=davem@davemloft.net \
    --cc=dingtianhong@huawei.com \
    --cc=ebiederm@xmission.com \
    --cc=justinvanwijngaarden@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=paul.gortmaker@windriver.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).