From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ajay Bhargav <ajay.bhargav@einfochips.com>
Date: Mon, 29 Aug 2011 10:40:49 +0530 (IST)
Subject: [U-Boot] [PATCH v3 1/3] net: Adds Fast Ethernet Controller
 driver for Armada100
In-Reply-To: <706935361.14309.1314594441316.JavaMail.root@ahm.einfochips.com>
Message-ID: <1181873733.14337.1314594649534.JavaMail.root@ahm.einfochips.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de


----- "Mike Frysinger" <vapier@gentoo.org> wrote:

> On Friday, August 26, 2011 02:36:51 Ajay Bhargav wrote:
> > +static int add_del_hash_entry(struct armdfec_device *darmdfec, u32
> mach,
> > +                              u32 macl, u32 rd, u32 skip, int del)
> > +{
> > +        u8 *last;
> 
> local var ...
> 
> > +        last = (u8 *) entry;
> > +        last = last + sizeof(*entry);
> > +
> > +        return 0;
> > +}
> 
> so what's the point of these two assignments to "last" ?
> 
I forgot to delete them during cleanup of initial code. Thanks for
pointing.

> > +int armada100_fec_register(int base_addr)
> 
> when it comes to addresses for memory mapped registers, we typically
> use 
> "unsigned long" rather than "int"
> 
yes right...

> > +        darmdfec = malloc(sizeof(struct armdfec_device));
> > +        if (!darmdfec)
> > +                goto error;
> 
> if this first one fails, we jump to:
> 
> > +error:
> > +        free(darmdfec->p_aligned_txbuf);
> > +        free(darmdfec->p_rxbuf);
> > +        free(darmdfec->p_rxdesc);
> > +        free(darmdfec->htpr);
> 
> looks like 4 NULL pointer derefs.  so you'll need one specific path
> for the 
> first malloc(), but the rest are fine.
> -mike
so you mean like this...

if(!darmdfec)
        goto error;
...
error1:
        free(darmdfec->p_aligned_txbuf);
        free(darmdfec->p_rxbuf);
        free(darmdfec->p_rxdesc);
        free(darmdfec->htpr);
error:
        free(darmdfec);
        return -1;

Thanks,
Ajay Bhargav