Just wanted to report that from the AIX side everything looks clean and ready to go.
Regards, Volker On Wed, Feb 17, 2016 at 5:56 PM, Seán Coffey <[email protected]> wrote: > One comment on the supportability side from me : > > java/net/AbstractPlainDatagramSocketImpl.java > >> + case SO_REUSEPORT: >> + if (o == null || !(o instanceof Boolean)) { >> + throw new SocketException("bad argument for >> SO_REUSEPORT"); >> + } >> + if >> (!supportedOptions().contains(StandardSocketOptions.SO_REUSEPORT)) { >> + throw new UnsupportedOperationException("unsupported >> option"); > > Can we improve the message ? e.g "SO_REUSEPORT is an unsupported option" - > line occurs twice. > > Ditto for java/net/AbstractPlainSocketImpl.java and > java/net/PlainDatagramSocketImpl.java and java/net/PlainSocketImpl.java > (windows & unix variants) > > > java/net/DualStackPlainDatagramSocketImpl.java, > java/net/DualStackPlainSocketImpl.java, java/net/PlainSocketImpl.java, > java/net/TwoStacksPlainDatagramSocketImpl.java and > java/net/TwoStacksPlainSocketImpl.java also need tweaking on that message. > > Thanks, > Sean. > > > On 17/02/16 11:43, Chris Hegarty wrote: >> >> >>> From: net-dev [mailto:[email protected]] On Behalf Of Lu, >>> Yingqi >>> Sent: Thursday, February 11, 2016 9:47 AM >>> To: Alan Bateman <[email protected]>; Volker Simonis >>> <[email protected]>; Michael McMahon <[email protected]> >>> Cc: Kaczmarek, Eric <[email protected]>; Viswanathan, Sandhya >>> <[email protected]>; Kharbas, Kishor <[email protected]>; >>> Aundhe, Shirish <[email protected]>; [email protected] >>> Subject: RE: Patch for adding SO_REUSEPORT socket option >>> >>> Hi Alan, >>> >>> Here is the most recent modification of the patch. The link is available >>> here: >>> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.11/ >> >> >> I know this has gone through several rounds of review already. >> I do not want to stall progress, I am reasonably happy with the >> changes, but I have a few comments ( that can be addressed later, >> if needed ). >> >> - MulticastSocket.java: I would add a javadoc link to >> {@linkplain StandardSocketOptions.SO_REUSEPORT SO_REUSEPORT} >> >> - Maybe the socket impl set/getOption(..) methods should check >> supportedOptions().contains(name) first. There seems to be >> many places where SO_REUSEPORT is special-cased, that could >> be replaced with a general supported options check, no? >> >> - How useful is it to add SO_REUSEPORT to non-listening stream >> orientated sockets ? >> >> -Chris. >> >> >> >>> Please let us know if everything is all right. >>> >>> Again, thank you very much for your help! >>> >>> Thanks, >>> Lucy >>> >>> >>> -----Original Message----- >>> From: Alan Bateman [mailto:[email protected]] >>> Sent: Wednesday, February 10, 2016 10:38 AM >>> To: Lu, Yingqi <[email protected]>; Volker Simonis >>> <[email protected]>; Michael McMahon <[email protected]> >>> Cc: [email protected]; Viswanathan, Sandhya >>> <[email protected]>; Kharbas, Kishor <[email protected]>; >>> Aundhe, Shirish <[email protected]>; Kaczmarek, Eric >>> <[email protected]> >>> Subject: Re: Patch for adding SO_REUSEPORT socket option >>> >>> On 05/02/2016 17:27, Lu, Yingqi wrote: >>>> >>>> Hi Alan, >>>> >>>> Here is the new modification of the patch. The link is available here: >>>> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.10/ >>>> >>>> In this version, we make supportedOptions() in AbstractPlainSocketImpl >>>> and AbstractPlainDatagramSocketImpl return an immutable set of the socket >>>> options. In addition, we corrected couple formatting issues. >>>> >>>> Please let us know your feedback and comments. >>>> >>> I've looked through the latest revision. Just a couple of small things: >>> >>> In MulticastSocket then a small typo (in two places) where you have >>> "SO_REUSPORT" instead of "SO_REUSEPORT". Also it links to >>> setOption(int,Object) then I assume it should be >>> setOption(SocketOption,Object). >>> >>> In AbstractPlainSocketImpl and AbstractPlainDatagramSocketImpl then >>> supportedOptions looks technically correct but there is no need to create an >>> unmodifiableSet on each call to supportedOptions. Instead you can simply do: >>> >>> Set<SocketOption<?>> options = socketOptions; if (options == null) { >>> if (isReusePortAvailable()) { >>> options = new HashSet<>(); >>> options.addAll(super.supportedOptions()); >>> options.add(StandardSocketOptions.SO_REUSEPORT); >>> options = Collections.unmodifiableSet(options); >>> } else { >>> options = super.supportedOptions(); >>> } >>> socketOptions = options; >>> } >>> return options; >>> >>> I don't see any other issues at this time and I'm happy to sponsor and >>> help you get this in. >>> >>> -Alan. >>> >>> >>> >
