Hi Chris, I have addressed your comments in a new webrev. Can you please have a look at this one?
http://cr.openjdk.java.net/~clanger/webrevs/8170868.3/ I created src/java.base/share/native/libnet/proxy_util.c/h and these files contain now the common used native parts. And I rewrote the code to return an array of Proxy objects from native code - so I do no longer call back to Java to add an object to the list. Best regards, Arno >-----Original Message----- >From: Chris Hegarty [mailto:chris.hega...@oracle.com] >Sent: Dienstag, 3. Januar 2017 15:04 >To: Zeller, Arno <arno.zel...@sap.com> >Cc: net-dev@openjdk.java.net >Subject: Re: RFR:8170868: DefaultProxySelector should use system defaults >on Windows, MacOS and Gnome > >Arno, > >> On 27 Dec 2016, at 11:44, Zeller, Arno <arno.zel...@sap.com> wrote: >> >> Hi Chris, >> >> Thanks for having a look at my change: >> >>> 1) It seems awful to have to deal with LinkedList in native code. How >>> about returning an array from native, and then converting that into >>> whatever list type is appropriate at the Java level. >> With the current implementation on Mac I do not know upfront how many >items the list will contain and therefore I preferred to use the LinkedList >from >native code. >> I can of course change the implementation on Mac to first generate a fully >resolved native list and then I can use NewObjectArray to generate an Array >of Proxy objects to return. This will avoid calling to java to add an element >to >the list. Do you think this will be better? > >Yes. Thanks. > >>> 2) I would prefer the use of List.of(...), and list.of() for empty, since >>> these are immutable and efficient list implementations. >> I thought about that but I tried to not return an immutable List because the >Javadoc of Proxy.select does not state anything about the returned List (if it >is >immutable or not) and I feared to break compatibility by returning an >immutable List. >> If you think this is no problem I will of course prefer to always return the >same immutable NO_PROXY list object. > >I would prefer an immutable list. Maybe we file, a separate, issue to amend >the spec to say "returns an immutable list …”. > >>> 3) Is the comment “Inspired ...” necessary / appropriate? >> I will change it to the comment proposed by Volker. > >Ok. > >>> 4) Can some of the native initialization code be moved to a platform >>> independent location, to remove duplication? >> Would it be ok if I move the definition of the static variables and the >implementation of 'static int initJavaClass(...)' to a header file in the >shared >branch. I.e. src/java.base/shared/native/libnet/DefaultProxySelector.h and >include this in the MacOSX, Windows and the Unix implementations? > >Yes. > >>> 5) The new file has a shared copyright header. I see similar SAP >>> headers in a few files, but none shared with the Oracle header. >>> How did you arrive at this format? >> The format was suggested to me by Volker :-) > >Ok. > >-Chris.
