Ivan, > On 26 Mar 2018, at 23:08, Ivan Gerasimov <ivan.gerasi...@oracle.com> wrote: > > Hello everyone! > > A few modifications were done to the patch. > Below is the list of updated parts of the patch with comments about what has > changed, comparing to the previous version of the patch. > > The patched JDK builds fine and all the tests pass well. > > WEBREV-000: http://cr.openjdk.java.net/~igerasim/8198358/04/000/webrev/
Ok. > WEBREV-001: http://cr.openjdk.java.net/~igerasim/8198358/04/001/webrev/ Ok. > WEBREV-002: http://cr.openjdk.java.net/~igerasim/8198358/04/002/webrev/ Can we make fdAccess final ( and in DualStack too ) static *final* JavaIOFileDescriptorAccess fdAccess JNIEXPORT void JNICALL Java_java_net_TwoStacksPlainSocketImpl_bind0 ... 142 if (IS_NULL(iaObj)) { 143 JNU_ThrowNullPointerException(env, "inet address argument"); 144 return; 145 } 146 147 family = getInetAddress_family(env, iaObj); 148 if (family != java_net_InetAddress_IPv4) { 149 JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", 150 "Protocol family not supported"); 151 return; 152 } The address is already null checked at the java level, so can be removed here, no? Can the family check be hoisted up into Java? I think this will help when it comes to the eventual merge with DualStack. The native code will then align with DualStack’s. I prefer to reduce and align the native layers as much as possible. It’s easier to deal with differences at the Java level. > WEBREV-003: http://cr.openjdk.java.net/~igerasim/8198358/04/003/webrev/ > - Rebased to reflect recent changes to socketListen() Ok. > WEBREV-004: http://cr.openjdk.java.net/~igerasim/8198358/04/004/webrev/ Ok. > WEBREV-005: http://cr.openjdk.java.net/~igerasim/8198358/04/005/webrev/ Ok. > WEBREV-006: http://cr.openjdk.java.net/~igerasim/8198358/04/006/webrev/ Ok. > WEBREV-007: http://cr.openjdk.java.net/~igerasim/8198358/04/007/webrev/ Ok. > WEBREV-008: http://cr.openjdk.java.net/~igerasim/8198358/04/008/webrev/ > - The check the line 400 was changed to test if (newfd < 0) and then if > (newfd == -2). > I don't think the later is quite accurate (cannot find in the documentation > that accept can return -2), but it is how it was done in TwoStacks > originally, so no regression is expected. Ok. 413 if (sa.sa.sa_family != AF_INET) { 414 JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", 415 "Protocol family not supported"); 416 NET_SocketClose(newfd); 417 return -1; 418 } This check could be hoisted up to the Java-level too, no? Again, I think it would be easier to deal with this kinda differences in Java. > WEBREV-009: http://cr.openjdk.java.net/~igerasim/8198358/04/009/webrev/ > - Added missing 'return -1;' at the line 188. Can the family be checked at the java level. > WEBREV-010: http://cr.openjdk.java.net/~igerasim/8198358/04/010/webrev/ Ok. > WEBREV-011: http://cr.openjdk.java.net/~igerasim/8198358/04/011/webrev/ Ok. > WEBREV-012: http://cr.openjdk.java.net/~igerasim/8198358/04/012/webrev/ Ok. > WEBREV-013: http://cr.openjdk.java.net/~igerasim/8198358/04/013/webrev/ Ok. > WEBREV-014: http://cr.openjdk.java.net/~igerasim/8198358/04/014/webrev/ Good. -Chris.
