Vyom, What about suggestion 1) below, the name of the socket option?
-Chris. > On 27 Sep 2017, at 09:56, vyom tewari <vyom.tew...@oracle.com> wrote: > > Hi Chris, > > Thanks for review, please find the latest > webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.2/index.html). I > incorporated review comments from you and re-base the patch to our > consolidated repo(jdk10/master). > > Thanks, > > Vyom > > > On Monday 25 September 2017 01:57 AM, Chris Hegarty wrote: >> Vyom, >> >>> On 11 Sep 2017, at 16:38, vyom tewari <vyom.tew...@oracle.com> wrote: >>> >>> Hi All, >>> >>> As jdk.net API already moved out of java.base, Please review the below code >>> change for jdk10. >>> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8145635 >>> Webrev: http://cr.openjdk.java.net/~vtewari/8145635/webrev0.1/index.html >>> >> This looks quite good. Some comments: >> >> 1) I wonder if we should just call the option TCP_QUICKACK, rather than >> SO_QUICKACK? Similar to TCP_NODELAY. >> >> 2) I think LinuxSocketOptions.h is largely unnecessary, if we do 1) above. >> >> 3) Java_jdk_net_LinuxSocketOptions_getQuickAck could return jint, rather >> than jobject, thus avoiding the need for createBoolean. The conversation can >> happen in the Java layer. Can you please reduce the lint length in this >> file, by wrapping similar to the style of the Solaris version. >> >> 4) ExtendedSocketOptions.java >> - drop the <p>, they are unnecessary. >> - I think, similar to TCP_NODELAY, the spec should say that the options is >> TCP specific. From TCP_NODELAY: "The socket option is specific to >> stream-oriented sockets using the TCP/IP protocol.” >> - "In TCP_QUICKACK mode”, maybe “When the option is enabled…” >> >> -Chris. >> >> >
