Review request: 7120875 fix macos ipv6 issue and update multiple test scripts

[Michael McMahon]

On 22/12/11 13:34, Alan Bateman wrote:
> On 22/12/2011 11:46, Michael McMahon wrote:
>>
>> I've updated this to localize the changes to NetworkInterface (and 
>> the new class DefaultInterface).
>> So, there's no change to the socket impl java code
>> The new native code implementation is in net_util_md.c also, but 
>> called from Plain*SocketImpl.c
>>> :
>> I've removed the debug outputs above. If all the above is ok, I'd 
>> like to push this change
>> and address remaining test case issues in another changeset.
>>
>> New webrev @ http://cr.openjdk.java.net/~michaelm/7120875/webrev.4/
>>
> This is much cleaner, thanks for persevering  with it. A couple of 
> minor comments:
>
Thanks Alan. I've no problem with the comments below. I will make the 
changes
and go with that version, unless there are other comments.

- Michael

> In NetworkInterface.java it looks a bit odd to add the static fields 
> in the middle of the other fields. It would also be nice to add a 
> comment to the package-private getDefault as it is used by several 
> classes (and we likely need to be used by DatagramChannel too once you 
> get to that area).
>
> In NetworkInterface.c then I assume the GetStaticFieldID doesn't need 
> ifdef MACOSX around it. It's likely it will get used on other 
> platforms too, Linux in particular.
>
> In PlainDatagramSocketImpl.c then I assume some of these changes could 
> be combined with the ifdef __solaris__, I don't have a strong opinion 
> on that.
>
> The Solaris and Windows implementations of DefaultInterface import 
> Enumeration and IOException, I assume they are not needed.
>
> The Mac version of DefaultInterface is bit untidy. The ifc, if1, if2 
> variables could be named differently. It looks like it may call 
> isPointToPoint and isLoopback several times which is costly given that 
> each method needs to query the system. I pasted in the static 
> initializer and edited here into a chooseDefaultInterface method that 
> I think is a bit tidier:
>
>     private final static NetworkInterface defaultInterface = 
> chooseDefaultInterface();
>
>     /**
>      * Choose a default interface. This method returns an interface 
> that is
>      * both "up" and supports multicast. This method choses an 
> interface in
>      * order of preference:
>      * 1. neither loopback nor point to point
>      * 2. point to point
>      * 3. loopback
>      *
>      * @return  the chosen interface or {@code null} if there isn't a 
> suitable
>      *          default
>      */
>     private static NetworkInterface chooseDefaultInterface() {
>         Enumeration<NetworkInterface> nifs;
>
>         try {
>            nifs = NetworkInterface.getNetworkInterfaces();
>         } catch (IOException ignore) {
>             // unable to enumate network interfaces
>             return null;
>         }
>
>         NetworkInterface ppp = null;
>         NetworkInterface loopback = null;
>
>         while (nifs.hasMoreElements()) {
>             NetworkInterface ni = nifs.nextElement();
>             try {
>                 if (ni.isUp() && ni.supportsMulticast()) {
>                     boolean isLoopback = ni.isLoopback();
>                     boolean isPPP = ni.isPointToPoint();
>                     if (!isLoopback && !isPPP) {
>                         // found an interface that is not the loopback 
> or a
>                         // point-to-point interface
>                         return ni;
>                     }
>                     if (ppp == null && isPPP)
>                         ppp = ni;
>                     if (loopback == null && isLoopback)
>                         loopback = ni;
>                 }
>             } catch (IOException skip) { }
>         }
>
>         return (ppp != null) ? ppp : loopback;
>     }
>
> The updates to the tests look fine to me.
>
> -Alan.
>