public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Dave Littell <littelld@verizon.net>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] Possible memory leak in cpu/ppc4xx/4xx_enet.c
Date: Mon, 17 Mar 2008 18:10:08 -0500	[thread overview]
Message-ID: <47DEFA50.4080601@verizon.net> (raw)
In-Reply-To: <200803170956.01571.sr@denx.de>

Stefan Roese wrote:
> On Saturday 15 March 2008, Dave Littell wrote:
>>>> I looked around this message and it seems that ppc_4xx_eth_initialize()
>>>> malloc()?s a EMAC_4XX_HW_PST structure and assigns it to dev->priv.
>>>> However, nothing ever frees this buffer so standing on a ping will
>>>> ultimately pull down all the available memory.
>>> Hmmm. IIRC ppc_4xx_eth_initialize() is only called once upon bootup for
>>> ethernet driver initialization. The function called each time before
>>> network action is ppc_4xx_eth_init().
>> It looks like ppc_4xx_eth_initialize() is called from eth_init() and
>> ppc_4xx_eth_init() is only called if there's an emac0_dev.
>>
>> Hm?
> 
> I see. You don't have CONFIG_NET_MULTI defined. Which 4xx platform are we 
> talking about btw?
> 

This is a platform similar to the AMCC sequoia, but CONFIG_NET_MULTI is
not defined.

>>>> One fix might be to free( dev->priv ) at the end of ppc_4xx_eth_halt().
>>>>  I tried this today and noted no ill effects.
>>> So did it work for you?
>> It seems to be working well.
> 
> OK. But your patch will most likely break all platforms with CONFIG_NET_MULTI 
> defined. Please give the attached patch a try and let me know if it works 
> too. I'll push it online if it fixes the problem.
> 

It does indeed seem to work - thanks!

> diff --git a/cpu/ppc4xx/4xx_enet.c b/cpu/ppc4xx/4xx_enet.c
> index d990250..479cb40 100644
> --- a/cpu/ppc4xx/4xx_enet.c
> +++ b/cpu/ppc4xx/4xx_enet.c
> @@ -1998,9 +1998,12 @@ int ppc_4xx_eth_initialize (bd_t * bis)
>  }
> 
>  #if !defined(CONFIG_NET_MULTI)
> -void eth_halt (void) {
> +void eth_halt (void)
> +{
>         if (emac0_dev) {
>                 ppc_4xx_eth_halt(emac0_dev);
> +               if (emac0_dev->priv)
> +                       free(emac0_dev->priv);
>                 free(emac0_dev);
>                 emac0_dev = NULL;
>         }
> 
> 

Thanks,
Dave

      reply	other threads:[~2008-03-17 23:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-15  2:44 [U-Boot-Users] Possible memory leak in cpu/ppc4xx/4xx_enet.c Dave Littell
2008-03-15  6:07 ` Stefan Roese
2008-03-15 15:12   ` Dave Littell
2008-03-17  8:56     ` Stefan Roese
2008-03-17 23:10       ` Dave Littell [this message]

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=47DEFA50.4080601@verizon.net \
    --to=littelld@verizon.net \
    --cc=u-boot@lists.denx.de \
    /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