RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl with TwoStacksPlainSocketImp [win]
Chris Hegarty
chris.hegarty at oracle.com
Tue Mar 27 13:47:26 UTC 2018
Ivan,
> On 26 Mar 2018, at 23:08, Ivan Gerasimov <ivan.gerasimov at 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.
More information about the net-dev
mailing list