What if eth_init() fails?

Hi all, I observed that when the Ethernet initialization fails, it is not properly halting the operation and exiting. On walking through the Networking files, I saw that the eth_init() in eth.c either returns a 0 or 1. Now, in the NetLoop() in net.c file, from where the eth_init() gets called, checks the condition if (eth_init(bd) < 0) { eth_halt(); return(-1); } which is thus never true. Thus the network operation, never exits gracefully, if Ethernet init fails. Any of you, have any clues about this??

Hi all, I observed that when the Ethernet initialization fails, it is not properly halting the operation and exiting. On walking through the Networking files, I saw that the eth_init() in eth.c either returns a 0 or 1. Now, in the NetLoop() in net.c file, from where the eth_init() gets called, checks the condition if (eth_init(bd) < 0) { eth_halt();

return(-1); } which is thus never true. Thus the network operation, never exits gracefully, if Ethernet init fails. Any of you, have any clues about this??
------------------------------------------------------------------------
View this message in context: What if eth_init() fails?
<http://www.nabble.com/What-if-eth_init%28%29-fails--tf4802433.html#a13740586>
Sent from the Uboot - Users mailing list archive
<http://www.nabble.com/Uboot---Users-f553.html> at Nabble.com.

Hi all, I observed that when the Ethernet initialization fails, it is not properly halting the operation and exiting. On walking through the Networking files, I saw that the eth_init() in eth.c either returns a 0 or 1. Now, in the NetLoop() in net.c file, from where the eth_init() gets called, checks the condition if (eth_init(bd) < 0) { eth_halt(); return(-1); } which is thus never true. Thus the network operation, never exits gracefully, if Ethernet init fails. Any of you, have any clues about this??

Quoted 8 lines · Upakul Barkakaty

Hi all, I observed that when the Ethernet initialization fails, it is not properly halting the operation and exiting. On walking through the Networking files, I saw that the eth_init() in eth.c either returns a 0 or 1. Now, in the NetLoop() in net.c file, from where the eth_init() gets called, checks the condition if (eth_init(bd) < 0) { eth_halt(); return(-1); } which is thus never true. Thus the network operation, never exits gracefully, if Ethernet init fails. Any of you, have any clues about this??

This is a known bug. The problem is that it's been around for so long, people don't realize what's happening. If you fix it, you might break something else.

so by fixing one bug, you may expose other bugs, and that's a bad thing ? bad code has gotta go !

Quoted 2 lines · Mike Frysinger

so by fixing one bug, you may expose other bugs, and that's a bad thing ?
bad code has gotta go !

My point was that if you fix this bug, you should be prepared for other bugs to show up, and people to be surprised about it. Don't just fix the first bug and walk away. If you can't test all the other platforms, then you should at least pay attention when other people report the other bugs, and then help them out.

Hi Upakul,

Quoted 1 line

Hi all, I observed that when the Ethernet initialization fails, it is

not

Quoted 1 line

properly halting the operation and exiting. On walking through the

Networking

Quoted 1 line

files, I saw that the eth_init() in eth.c either returns a 0 or 1. Now,

in the

Quoted 1 line

NetLoop() in net.c file, from where the eth_init() gets called, checks

the

Quoted 1 line

condition if (eth_init(bd) < 0) { eth_halt(); return(-1); } which is

thus never

Quoted 1 line

true. Thus the network operation, never exits gracefully, if Ethernet

init

Quoted 1 line

fails. Any of you, have any clues about this??

I think your analysis is right and thus the code in NetLoop should be fixed. Feel free to send a patch and I am pretty sure Ben Warren will pick it up into the u-boot-net repo.

Cheers
Detlev

--
Two monks went fishing in an electron river. The first monk drew out his
network, and out flopped a hacker. The second monk cried, "The poor hacker!
How can it live outside of the network?" The first monk said, "When
you
have learned to live outside the network, then you will know." --
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Thanks for the replies. I am attaching herewith, the patch which I suppose should fix the issue in NetLoop().

Quoted 9 lines

net/net.c +1 -1

@@ -305,7 +305,7 @@
 #ifdef CONFIG_NET_MULTI
 	eth_set_current();
 #endif
-	if (eth_init(bd) < 0) {
+	if (eth_init(bd) > 0) {
 		eth_halt();

net/net.c line 311

Quoted 2 lines

net/net.c +0 -0

 		return(-1);
 	}

Secondly and more important, did you test this? I'd say your test is the wrong way round, i.e. eth_init returns true in the C sense (!=0) if it was able to initialize an interface. (This also chimes with the naming of the function by the way). So I'd propose to go for "!eth_init(..)".

Thanks for the replies. I am attaching herewith, the patch which I suppose should fix the issue in NetLoop().