Re: Code Review 6972374: NetworkInterface.getNetworkInterfaces throws "java.net.SocketException" on Solaris zone
Chris Hegarty wrote: : Thanks, I removed it. Updated webrev: http://cr.openjdk.java.net/~chegar/6972374/webrev.00/webrev/ The updated webrev looks good to me. : I would be hopeful that there wouldn't be any more issues arising from 6931566 now. As you said, it's also difficult to test the various different types of configurations. What made reviewing the changes for 6931566 difficult is that there was a lot of refactoring in the code. I believe this makes the code much more readably, but the down side is these kind of issues. I agree it's much more readable, just concerned that there are other issues lurking. -Alan.
Re: Code Review 6972374: NetworkInterface.getNetworkInterfaces throws "java.net.SocketException" on Solaris zone
On 07/28/10 10:25, Alan Bateman wrote: Chris Hegarty wrote: Dmitry, Alan, The Solaris version of getFlags sets an Exception if the ioctl fails. When used in addif getFlags will fail when access to the virtual interface's parent is forbidden, i.e. in a zone. addif is called when iterating over interfaces in enumIPvXInterfaces, if an exception occurs it simply cleans up and returns, propagating the exception. getFalgs should not set an exception. All other calls to it check the return value and set an exception if appropriate. Webrev: http://cr.openjdk.java.net/~chegar/6972374/webrev.00/webrev/ Thanks, -Chris. Looks okay to me. I assume the JNIEnv parameter is no longer needed. Thanks, I removed it. Updated webrev: http://cr.openjdk.java.net/~chegar/6972374/webrev.00/webrev/ It seems to me that we've had a slew of issues with NetworkInterface lately. Would it be worth re-reviewing the changes for 6931566 in case there are more? Ideally we should improve the test coverage to catch more issues but NetworkInterface is tricky to test completely due to variety of configurations in the wild. I would be hopeful that there wouldn't be any more issues arising from 6931566 now. As you said, it's also difficult to test the various different types of configurations. What made reviewing the changes for 6931566 difficult is that there was a lot of refactoring in the code. I believe this makes the code much more readably, but the down side is these kind of issues. -Chris. -Alan.
Re: Code Review 6972374: NetworkInterface.getNetworkInterfaces throws "java.net.SocketException" on Solaris zone
Chris Hegarty wrote: Dmitry, Alan, The Solaris version of getFlags sets an Exception if the ioctl fails. When used in addif getFlags will fail when access to the virtual interface's parent is forbidden, i.e. in a zone. addif is called when iterating over interfaces in enumIPvXInterfaces, if an exception occurs it simply cleans up and returns, propagating the exception. getFalgs should not set an exception. All other calls to it check the return value and set an exception if appropriate. Webrev: http://cr.openjdk.java.net/~chegar/6972374/webrev.00/webrev/ Thanks, -Chris. Looks okay to me. I assume the JNIEnv parameter is no longer needed. It seems to me that we've had a slew of issues with NetworkInterface lately. Would it be worth re-reviewing the changes for 6931566 in case there are more? Ideally we should improve the test coverage to catch more issues but NetworkInterface is tricky to test completely due to variety of configurations in the wild. -Alan.
Re: Code Review 6972374: NetworkInterface.getNetworkInterfaces throws "java.net.SocketException" on Solaris zone
Chris, Looks good for me. -Dmitry On 2010-07-28 12:25, Chris Hegarty wrote: Dmitry, Alan, The Solaris version of getFlags sets an Exception if the ioctl fails. When used in addif getFlags will fail when access to the virtual interface's parent is forbidden, i.e. in a zone. addif is called when iterating over interfaces in enumIPvXInterfaces, if an exception occurs it simply cleans up and returns, propagating the exception. getFalgs should not set an exception. All other calls to it check the return value and set an exception if appropriate. Webrev: http://cr.openjdk.java.net/~chegar/6972374/webrev.00/webrev/ Thanks, -Chris. -- Dmitry Samersoff J2SE Sustaining team, SPB04 * Give Rabbit time and he'll always get the answer ...
Code Review 6972374: NetworkInterface.getNetworkInterfaces throws "java.net.SocketException" on Solaris zone
Dmitry, Alan, The Solaris version of getFlags sets an Exception if the ioctl fails. When used in addif getFlags will fail when access to the virtual interface's parent is forbidden, i.e. in a zone. addif is called when iterating over interfaces in enumIPvXInterfaces, if an exception occurs it simply cleans up and returns, propagating the exception. getFalgs should not set an exception. All other calls to it check the return value and set an exception if appropriate. Webrev: http://cr.openjdk.java.net/~chegar/6972374/webrev.00/webrev/ Thanks, -Chris.
