On 03/09/13 10:45, Dmitry Samersoff wrote:
Mark,
1. If I read the code correctly,
ll. 168 - 177 is exactly the same as ll. 180 - 189
Could you refactor the fix to avoid code duplication?
Would something like
if (
(ptr->IfIndex != 0 && ptr->IfIndex == index) ||
(ptr->Ipv6IfIndex != 0 && ptr->Ipv6IfIndex == index)
) {
...
}
work?
I think this makes sense.
Also, I'm not sure I understand the purpose of the change
at line 267 (old) 281 (new). What the old code seemed to be doing
was interpreting an IPv6 index as actually an IPv4 one.
Was that not correct? This code is rather brittle unfortunately
and not the easiest to understand.
Michael
2. It seems to me passed index couldn't be 0 so null check is redundant,
but I don't mind to keep it.
It might make sense to explicitly check passed index for 0 above loop.
-Dmitry
On 2013-09-03 13:23, Mark Sheppard wrote:
Hi
please oblige and review the fix below to address the issue in JDK-8021372:
NetworkInterface.getNetworkInterfaces() returns duplicate hardware address
http://cr.openjdk.java.net/~msheppar/8021372/webrev/
the handling of the Ipv6IfIndex was suspect when setting the
interface index and when retrieving the mac address using the index.
in the getAdpaters function, the conditaionalstatement could be rolled
into one.
But, as there was a spurious IPv4 comment, I structured it as an if
else. statement.
It should be noted that IfIndex and the Ipv6IfIndex can be zero, which
implies that the
IPv4 or IPv6 interface is not available. This may be at odds with the
setting of a default index to 0 in the Java NetworkInterface abstraction,
where a defaultIndex of zero is used.
JPRTs have been run and show no side effects.
regards
Mark
