I have verified this commit. Thanks Alan, Chris and Charles!
On 05/11/2012 04:25 PM, Charles Lee wrote:
Hi Deven,
The patch is committed @
Changeset: c5a07e3dca63
Author: youdwei
Date: 2012-05-11 16:20 +0800
URL:http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c5a07e3dca63
7163874: InetAddress.isReachable should support pinging 0.0.0.0
Reviewed-by: alanb, chegar
Please verify it and thank you all for reviewing it.
On 05/09/2012 11:36 AM, Deven You wrote:
Hi Alan,
Since this patch is for Unix like platforms, I didn't test the test
case on Windows yet.
However when I tested it on Windows I found it will fail on Windows.
I also use ping command on Windows to test both 0.0.0.0 and ::0, they
all fail. So I want to know if we need further investigation to see
why these 2 addresses can not be reachable or we just think it is the
proper behavior because JDK's behavior is consistent with ping on
Windows?
Thanks a lot!
On 05/07/2012 03:46 PM, Alan Bateman wrote:
On 07/05/2012 03:29, Deven You wrote:
Hi Alan and Chris,
I have updated the webrev[1] according to your suggestions.
For the test case, I did two things:
1. add the @run main/othervm -Djava.net.preferIPv4Stack=true PingThis
2. Only when !preferIPv4Stack and hasIPv6(), ::0 will be added into
the address list.
I think it's enough for the test case, please review it.
I just have another concern, how we can configure the test env so
this test case can be run under root (may use sudo) privilege?
[1] http://cr.openjdk.java.net/~littlee/OJDK-217/webrev.04/
<http://cr.openjdk.java.net/%7Elittlee/OJDK-217/webrev.04/>
I think we are close to the finish line on this one.
One thing I notice in the webrev is that net_util_md.h hasn't been
updated to defined the prototype for NET_IsZeroAddr. It would be
good to add that (no need to generate another webrev just for this one).
The test case looks okay to me. Minor comment is that the while loop
could be replaced with for (String: addr: addrs) { ...}. No need to
re-generate the webrev if you decide to take up this suggestion.
I don't think we should change the test to attempt to run it via
sudo, that would just complicate the test as there is no guarantee
that the user running the tests is in sudoers. Also one wouldn't
want the test prompting for a password. it would also require a
script as it would need to be skipped on platforms such as Windows.
So I think I'm happy with the test as is and I assume you will run
with regular user/root before pushing. I also assume you will run it
on Windows to make sure that it passes there too.
-Alan.
--
Best Regards,
Deven
--
Yours Charles
--
Best Regards,
Deven
