Skip to contentSkip to conversation

net: Adds Fast Ethernet Controller driver for Armada100

PATCH Superseded Ajay Bhargav 3 participants 6 messages Aug 29, 2011 – Aug 30, 2011 1 message not in the archive

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ajay Bhargav

author
#1 lore raw

PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100

Quoted 9 lines · ----- "Mike Frysinger
Quoted 1 line · On Friday, August 26, 2011 02:36:51 Ajay Bhargav wrote:

+static int add_del_hash_entry(struct armdfec_device *darmdfec, u32

mach,

Review

Quoted 3 lines
(quoted diff) +3 -0
+                              u32 macl, u32 rd, u32 skip, int del)
+{
+        u8 *last;

local var ...

Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+        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.

Quoted 6 lines
Quoted 1 line

+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...

Quoted 12 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+        darmdfec = malloc(sizeof(struct armdfec_device));
+        if (!darmdfec)
+                goto error;

if this first one fails, we jump to:

Review

Quoted 5 lines
(quoted diff) +5 -0
+error:
+        free(darmdfec->p_aligned_txbuf);
+        free(darmdfec->p_rxbuf);
+        free(darmdfec->p_rxdesc);
+        free(darmdfec->htpr);
Quoted 4 lines

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

#2 lore raw

PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100

Quoted 19 lines · On Monday, August 29, 2011 01:10:49 Ajay Bhargav wrote:
Quoted 6 lines · ----- "Mike Frysinger

Review

Quoted 3 lines · On Friday, August 26, 2011 02:36:51 Ajay Bhargav wrote:
(quoted diff) +3 -0
+        darmdfec = malloc(sizeof(struct armdfec_device));
+        if (!darmdfec)
+                goto error;

if this first one fails, we jump to:

Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+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.

so you mean like this...

if(!darmdfec)

Quoted 10 lines
        goto error;
...
error1:
        free(darmdfec->p_aligned_txbuf);
        free(darmdfec->p_rxbuf);
        free(darmdfec->p_rxdesc);
        free(darmdfec->htpr);
error:
        free(darmdfec);
        return -1;

that's one way of fixing it
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110829/dccb11cf/attachment.pgp

author
#3 lore raw

PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100

Quoted 32 lines · ----- "Mike Frysinger
Quoted 6 lines · On Monday, August 29, 2011 01:10:49 Ajay Bhargav wrote:
Quoted 5 lines · ----- "Mike Frysinger
Quoted 3 lines · On Friday, August 26, 2011 02:36:51 Ajay Bhargav wrote:
(quoted diff) +3 -0
+        darmdfec = malloc(sizeof(struct armdfec_device));
+        if (!darmdfec)
+                goto error;
Quoted 8 lines
Quoted 8 lines

if this first one fails, we jump to:

Review

Quoted 5 lines
(quoted diff) +5 -0
+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

Quoted 5 lines
Quoted 1 line

for the first malloc(), but the rest are fine.

so you mean like this...

if(!darmdfec)

Quoted 10 lines
        goto error;
...
error1:
        free(darmdfec->p_aligned_txbuf);
        free(darmdfec->p_rxbuf);
        free(darmdfec->p_rxdesc);
        free(darmdfec->htpr);
error:
        free(darmdfec);
        return -1;
Quoted 2 lines

that's one way of fixing it
-mike

Any other better way?

--Ajay

#4 lore raw

PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100

Quoted 35 lines · Ajay Bhargav
Quoted 34 lines · ----- "Mike Frysinger
Quoted 6 lines · On Monday, August 29, 2011 01:10:49 Ajay Bhargav wrote:
Quoted 5 lines · ----- "Mike Frysinger
Quoted 3 lines · On Friday, August 26, 2011 02:36:51 Ajay Bhargav wrote:
(quoted diff) +3 -0
+        darmdfec = malloc(sizeof(struct armdfec_device));
+        if (!darmdfec)
+                goto error;
Quoted 8 lines
Quoted 8 lines

if this first one fails, we jump to:

Review

Quoted 5 lines
(quoted diff) +5 -0
+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

Quoted 16 lines
Quoted 1 line

for the first malloc(), but the rest are fine.

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);
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

error:

        free(darmdfec);
        return -1;

that's one way of fixing it
-mike

Any other better way?

I'd love to be enlightened on this topic too ...

Quoted 6 lines

--Ajay
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#5 lore raw

PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100

In reply to Ajay Bhargav
Quoted 35 lines · On Monday, August 29, 2011 01:39:18 Ajay Bhargav wrote:
Quoted 34 lines · ----- "Mike Frysinger
Quoted 6 lines · On Monday, August 29, 2011 01:10:49 Ajay Bhargav wrote:
Quoted 5 lines · ----- "Mike Frysinger
Quoted 3 lines · On Friday, August 26, 2011 02:36:51 Ajay Bhargav wrote:
(quoted diff) +3 -0
+        darmdfec = malloc(sizeof(struct armdfec_device));
+        if (!darmdfec)
+                goto error;
Quoted 8 lines
Quoted 8 lines

if this first one fails, we jump to:

Review

Quoted 5 lines
(quoted diff) +5 -0
+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

Quoted 16 lines
Quoted 1 line

for the first malloc(), but the rest are fine.

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);
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

error:

        free(darmdfec);
        return -1;

that's one way of fixing it

Any other better way?

since all your buffers are known fixed (at compile time) sizes, you could simply inline them into the structure so you only malloc() one buffer -mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110829/fd59d9d9/attachment.pgp

author
#6 lore raw

PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100

Quoted 41 lines · ----- "Mike Frysinger
Quoted 8 lines · On Monday, August 29, 2011 01:39:18 Ajay Bhargav wrote:
Quoted 7 lines · ----- "Mike Frysinger
Quoted 6 lines · On Monday, August 29, 2011 01:10:49 Ajay Bhargav wrote:
Quoted 5 lines · ----- "Mike Frysinger
Quoted 3 lines · On Friday, August 26, 2011 02:36:51 Ajay Bhargav wrote:
(quoted diff) +3 -0
+        darmdfec = malloc(sizeof(struct armdfec_device));
+        if (!darmdfec)
+                goto error;
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

if this first one fails, we jump to:

Review

Quoted 5 lines
(quoted diff) +5 -0
+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

Quoted 23 lines
Quoted 23 lines

path

Quoted 19 lines
Quoted 1 line

for the first malloc(), but the rest are fine.

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;
Quoted 8 lines
Quoted 3 lines
Quoted 1 line

that's one way of fixing it

Any other better way?

since all your buffers are known fixed (at compile time) sizes, you could
simply inline them into the structure so you only malloc() one buffer -mike

Yes.. thats another way of coding :) I will stick to previous one..

Thanks,
Ajay Bhargav