Re: [Etherboot-developers] Uninitialized 'iplen' in nic.c

[Robb M. <ma...@ac...>]

> please use diff -u next time.  It is a whole lot more readable.

Thanks for the pointer, new patch attached.

> I have a big problem with the patch.  It does exactly the opposite
> of what it is supposed to do.  It adds a case where iplen will
> be used initialized.  In particular you take out the surrounding
> test for ip.  You cannot have a udp unless it is also an ip packet.
> But you can sure have a packet that is not a ip packet.

I hope the problem is due to the difficulty in readability, and not because
I'm missing something...
I originally looked into the issue because the compiler was complaining
about iplen potentially being used without being initialized. With the patch
applied, this warning is gone.

I know that the UDP is encapsulated in an IP packet, and the patch changes
the nesting so that the check for UDP occurs only for packets where (ptype
== IP), and hence the check for ip nonzero is no longer necessary in the
check for UDP packets. Originally, ip was set to zero, then set non-zero
within the (ptype==IP) case, and was a precondition for checking for
(ip->protocol == IP_UDP). Instead, the patch simply makes the check for UDP
a subset of the if{} that checks for (ptype == IP). Functionally equivalent,
a whole lot clearer, and it stops the compiler complaining - I usually take
"uninitialized variable" compiler warnings seriously.

The if{} nesting is not well illustrated in the patch - I will send you a
gzip of the patched nic.c privately for review.

> Is this just code motion or is more involved.  We are approaching
> a stable release, and I just don't want to see things break
> at this late stage in the game.

This is code motion - should not be introducing any breakage :-)
The intention is to support the memtest86 usage I mentioned earlier.

Robb.