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

Alan Bateman Alan.Bateman at oracle.com
Thu Dec 22 05:34:39 PST 2011


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:

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.



More information about the macosx-port-dev mailing list