hg: jdk8/tl/jdk: 2 new changesets

2013-10-03 Thread roger . riggs
Changeset: 01b8604e8268 Author:rriggs Date: 2013-08-22 10:01 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/01b8604e8268 8024896: Refactor java.time serialization tests into separate subpackage Summary: Move serialization tests to .serial subpackage Reviewed-by: sherman Contr

hg: jdk8/tl/jdk: 7 new changesets

2013-10-04 Thread roger . riggs
Changeset: 1de0fac9b962 Author:rriggs Date: 2013-08-29 20:38 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1de0fac9b962 8023764: Optimize Period addition Summary: Optimise plus/minus for common cases Reviewed-by: sherman Contributed-by: scolebou...@joda.org ! src/share/clas

hg: jdk8/tl/jdk: 2 new changesets

2013-10-09 Thread roger . riggs
Changeset: e3b70e601c1c Author:rriggs Date: 2013-10-09 11:02 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e3b70e601c1c 8024612: java/time/tck/java/time/format/TCKDateTimeFormatters.java failed Summary: The test should be using the Locale.Category.FORMAT to verify the test

hg: jdk8/tl/jdk: 8024076: Incorrect 2 -> 4 year parsing and resolution in DateTimeFormatter

2013-10-09 Thread roger . riggs
Changeset: d42fe440bda8 Author:rriggs Date: 2013-10-09 13:34 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d42fe440bda8 8024076: Incorrect 2 -> 4 year parsing and resolution in DateTimeFormatter Summary: Add appendValueReduced method based on a ChronoLocalDate to provide co

hg: jdk8/tl/jdk: 3 new changesets

2013-10-15 Thread roger . riggs
Changeset: ea422834f880 Author:rriggs Date: 2013-09-26 23:05 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ea422834f880 8025720: Separate temporal interface layer Summary: Remove ZoneId and Chronology from TemporalField interface Reviewed-by: sherman Contributed-by: scolebou

hg: jdk8/tl/jdk: 8026516: javadoc errors in java.time

2013-10-17 Thread roger . riggs
Changeset: 36fe6a9bd43e Author:rriggs Date: 2013-10-17 10:37 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/36fe6a9bd43e 8026516: javadoc errors in java.time Summary: Corrected links to TemporalQuery and TemporalField.resolve Reviewed-by: mduigou, darcy, lancea ! src/share/c

hg: jdk8/tl/jdk: 8026183: minor documentation problems in java.lang.invoke; ...

2013-10-17 Thread roger . riggs
Changeset: 456a9b199208 Author:rriggs Date: 2013-10-17 13:43 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/456a9b199208 8026183: minor documentation problems in java.lang.invoke 8015808: Typo in MethodHandle javadoc Summary: Fix typos and javadoc markup and extraneous paragr

hg: jdk8/tl/jdk: 8025828: Late binding of Chronology to appendValueReduced

2013-10-18 Thread roger . riggs
Changeset: 7a947daa8f51 Author:rriggs Date: 2013-10-18 16:37 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7a947daa8f51 8025828: Late binding of Chronology to appendValueReduced Summary: Add a listener to the parseContext called when the Chronology changes Reviewed-by: sherm

hg: jdk8/tl/jdk: 3 new changesets

2013-10-22 Thread roger . riggs
Changeset: e2b814e68956 Author:rriggs Date: 2013-10-22 15:03 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e2b814e68956 8024686: Cleanup of java.time serialization source Summary: optimize serialized form of OffsetTime, OffsetDateTime; correct order of modifiers Reviewed-by

hg: jdk8/tl/jdk: 8026982: javadoc errors in core libs

2013-10-22 Thread roger . riggs
Changeset: 4bb758a77fd7 Author:rriggs Date: 2013-10-22 17:02 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4bb758a77fd7 8026982: javadoc errors in core libs Summary: Cleanup of javadoc -Xlint errors Reviewed-by: lancea, mduigou, darcy, mullan, mchung ! src/share/classes/jav

Re: RFR: Inet[4|6]Address native initializing code should check fieldID values

2013-11-05 Thread roger riggs
H Michael, If I remember rightly, CHECK_NULL tests for an exception and conditionally does an early return from the function. So testing just before an unconditional return is unnecessary. (These look ok to me; not a Reviewer) Roger On 11/5/2013 3:39 PM, Michael McMahon wrote: On 05/11/13

hg: jdk8/tl/jdk: 8024458: DataInput.readDouble refers to "readlong" instead of "readLong"

2013-11-07 Thread roger . riggs
Changeset: 04f071a95c29 Author:rriggs Date: 2013-11-07 20:56 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/04f071a95c29 8024458: DataInput.readDouble refers to "readlong" instead of "readLong" Summary: fix the typo Reviewed-by: lancea, chegar, dxu ! src/share/classes/java/i

Re: RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)

2013-11-08 Thread roger riggs
Hi, Does renaming the file/directory suffer the same delay? I could see a cleanup mechanism that renames them to hidden files (or entirely out of the work directory) and then deletes them. That would immediately clear the namespace for tests to proceed. That technique should work on all pla

hg: jdk8/tl/jdk: 8028041: Serialized Form description of j.l.String is not consistent with the implementation

2013-11-08 Thread roger . riggs
Changeset: df2f7f288353 Author:rriggs Date: 2013-11-08 17:50 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/df2f7f288353 8028041: Serialized Form description of j.l.String is not consistent with the implementation Summary: Replaced incorrect description with reference to the

hg: jdk8/tl/jdk: 8028092: Lint cleanup of java.time.format

2013-11-09 Thread roger . riggs
Changeset: 3add16c86970 Author:rriggs Date: 2013-11-09 14:30 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3add16c86970 8028092: Lint cleanup of java.time.format Summary: correct declarations and add @SuppressWarnings Reviewed-by: darcy, lancea ! src/share/classes/java/time

hg: jdk8/tl/jdk: 8028014: Doclint warning/error cleanup in javax.management

2013-11-12 Thread roger . riggs
Changeset: ebe27e1a2e2d Author:rriggs Date: 2013-11-12 14:03 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ebe27e1a2e2d 8028014: Doclint warning/error cleanup in javax.management Summary: Improve generated html by fixing doclint warnings Reviewed-by: sla, jbachorik ! src/sh

hg: jdk8/tl/jdk: 8028141: test/sun/management/jmxremote/bootstrap/LocalManagementTest|CustomLauncherTest.java failing again

2013-11-19 Thread roger . riggs
Changeset: 3f47e393e1dd Author:rriggs Date: 2013-11-19 13:20 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3f47e393e1dd 8028141: test/sun/management/jmxremote/bootstrap/LocalManagementTest|CustomLauncherTest.java failing again Summary: Correct to use the test.class.path in

hg: jdk8/tl/jdk: 8028019: AWT Doclint warning/error cleanup

2013-12-03 Thread roger . riggs
Changeset: 3e95aadb479f Author:rriggs Date: 2013-12-03 16:20 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3e95aadb479f 8028019: AWT Doclint warning/error cleanup Summary: Fix numerious javadoc and html errors and warnings Reviewed-by: yan ! src/share/classes/java/applet/Ap

hg: jdk8/tl/jdk: 8029551: Add value-type notice to java.time classes

2013-12-11 Thread roger . riggs
Changeset: fe3383582427 Author:rriggs Date: 2013-12-11 16:52 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/fe3383582427 8029551: Add value-type notice to java.time classes Summary: Add warning about identity of value types and reference to ValueBased.html Reviewed-by: brian

Re: RFR: simple javadoc cleanup in java.net

2013-12-18 Thread roger riggs
Thanks Alan, I corrected the webrev to link both locations that refer to ProxySelector. http://cr.openjdk.java.net/~rriggs/webrev-javadoc-link-7018010/ Roger On 12/18/2013 6:23 AM, Alan Bateman wrote: On 17/12/2013 22:35, roger riggs wrote: Please review, an easy one, just 1 less bug on

hg: jdk8/tl/jdk: 2 new changesets

2013-12-20 Thread roger . riggs
Changeset: 7186275e6ef1 Author:rriggs Date: 2013-12-20 13:06 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7186275e6ef1 8030002: Enhance deserialization using readObject Reviewed-by: sherman, chegar, scolebourne ! src/share/classes/java/time/Duration.java ! src/share/classe

hg: jdk8/tl/jdk: 8031103: java.time.Duration has wrong Javadoc Comments in toDays() and toHours()

2014-01-07 Thread roger . riggs
Changeset: 1b503dd54b95 Author:rriggs Date: 2014-01-07 11:50 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1b503dd54b95 8031103: java.time.Duration has wrong Javadoc Comments in toDays() and toHours() Summary: Correct specification for Duration.toDays, toHours Reviewed-by: l

RFR: (8030875) Macros for checking and returning on exceptions

2014-01-10 Thread roger riggs
Please review: To enable native code checking consistently for thrown exceptions, the macros in net_util.h and java/util/jar/pack/coding.cpp are made consolidated and promoted to jni_util.h webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ [1] https://bugs.openjdk.java.

Re: RFR: (8030875) Macros for checking and returning on exceptions

2014-01-10 Thread roger riggs
Hi Mandy, Good point; I'll create another issue to do that update. (I should have waited a bit longer for comments.) Roger On 1/10/2014 12:41 PM, Mandy Chung wrote: On 1/10/2014 7:37 AM, roger riggs wrote: Please review: To enable native code checking consistently for thrown exceptions

Re: RFR: (8030875) Macros for checking and returning on exceptions

2014-01-14 Thread roger riggs
Hi David, The CHECK_RETURN macros have existed in java.net for some time and I have not seen any empty statement warnings. The CHECK_EXCEPTION macros are new and does not have any uses yet. I don't plan to do any wholesale modification of current sources. The macros always produce a valid state

RFR: (8031737) rename jni_util.h macros for checking and returning on exceptions

2014-01-16 Thread roger riggs
Please review: The native macros for checking and returning when exceptions are thrown have been renamed to include the "JNU_" prefix consistent with other functions in jni_util.h. The macros have been renamed and existing uses in the jdk repository for networking, pack200, and have been updated

Re: RFR: (8031737) rename jni_util.h macros for checking and returning on exceptions

2014-01-16 Thread roger riggs
wrote: On 16/01/2014 16:26, roger riggs wrote: Please review: The native macros for checking and returning when exceptions are thrown have been renamed to include the "JNU_" prefix consistent with other functions in jni_util.h. The macros have been renamed and existing uses in the jdk repo

Re: RFR: (8031737) rename jni_util.h macros for checking and returning on exceptions

2014-01-16 Thread roger riggs
nvolve jni. Roger On 1/16/2014 11:41 AM, Alan Bateman wrote: On 16/01/2014 16:26, roger riggs wrote: Please review: The native macros for checking and returning when exceptions are thrown have been renamed to include the "JNU_" prefix consistent with other functions in jni_util.h.

Re: RFR: (8031737) rename jni_util.h macros for checking and returning on exceptions

2014-01-16 Thread roger riggs
ithout being used on a method that involves jni. An overly aggressive find caught the uses in java/util/jar/pack. But yes, it might be better to limit their scope to functions that involve jni. Roger On 1/16/2014 11:41 AM, Alan Bateman wrote: On 16/01/2014 16:26, roger riggs wrote:

Re: RFR: (8031737) rename jni_util.h macros for checking and returning on exceptions

2014-01-16 Thread roger riggs
The webrev has been updated to revert the java.util.jar/pack CHECK_* macros and to clean up the macro definitions in jni_util.h. I plan to give the review some more time in case there are more comments coming. Roger On Thu, Jan 16, 2014 at 8:26 AM, roger riggs <mailto:roger.ri...@oracle.

Re: RFR: 8041397: Lint regression in java.net.SocketOption

2014-04-23 Thread roger riggs
Hi Michael, Looks fine. Roger On 4/23/2014 11:56 AM, Michael McMahon wrote: Could I get the following small change reviewed please? It fixes a number of recently introduced lint warnings http://cr.openjdk.java.net/~michaelm/8041397/webrev.1/ Thanks Michael.

Re: RFR(S): 8055032: Improve numerical parsing in java.net and sun.net

2014-08-14 Thread roger riggs
Hi, My preference would be to keep the offsets immediately following the CharSequence, that clearly identifies the substring being operated on. Roger On 8/14/2014 8:00 AM, Alan Bateman wrote: On 14/08/2014 12:42, Claes Redestad wrote: Noone brought it up, as far as I can recall. Since par

Re: RFR: 8067680: (sctp) Possible race initializing native IDs

2015-01-28 Thread Roger Riggs
Looks good. Roger On 1/28/2015 11:46 AM, Rob McKenna wrote: This is a fix to a possible race in the current sctp code. In a nutshell the conditional only checks that isaCls is not null. However if isaCls is set by one thread and that thread is interrupted before setting isaCtrID, the interrup

Re: RFR: JDK-8046901 - Check jdk/src/solaris/native/sun/nio for Parfait flagged uninitialized memory

2015-02-18 Thread Roger Riggs
Hi, Looks fine to me also. Unfortunately, marking false positives in the tool creates a required synchronization between the tool configuration and the JDK, if that gets lost or out of sync the warnings might re-appear. In theory, it might be possible to inform the tool of what happens to the

Re: WebSocket client API

2015-09-01 Thread Roger Riggs
Hi Pavel, A few comments on the API: - The Incoming class combines separate functions that would be easier to use if the methods were directly on the Builder. It would also allow (not require) lambda's to be used or method references if the code was complex. The Builder methods can

Re: WebSocket client API

2015-09-03 Thread Roger Riggs
Hi Pavel, On 9/2/2015 4:40 PM, Pavel Rappo wrote: Hi Roger, thanks for looking at this! On 1 Sep 2015, at 17:05, Roger Riggs wrote: - The Incoming class combines separate functions that would be easier to use if the methods were directly on the Builder. ... |WebSocket ws

Re: SO_REUSEPORT feature support in JDK 9 for socket communication

2015-10-22 Thread Roger Riggs
Hi Sandhya, The folks on net-dev@openjdk.java.net will be interested too. Roger On 10/21/2015 9:08 PM, Viswanathan, Sandhya wrote: This is a proposal for adding SO_REUSEPORT support in JDK 9 for socket communication. The feature is supported since Linux Kernel 3.9 in OS. It is also support

Re: ServerImpl misplaced null check

2016-01-20 Thread Roger Riggs
Looks like a bug to me. Network issues are better raised on net-dev@openjdk.java.net Created: 8147862 Null check too late in sun.net.httpserver.ServerImpl Thanks, Roger On 1/20/2016 1:18 PM, Scott Palmer wrote: I was searching for a way to s

Re: RFR: 8087112 HTTP API and HTTP/1.1 implementation

2016-02-17 Thread Roger Riggs
Hi Michael, Looks great! With a new package, the doc will be important to help get developers started. The package-info.java should help a bit more getting started. The examples in the HttpRequest are very informative, either the package-java should show an example or be more explicit that

Re: RFR: 8087112 HTTP API and HTTP/1.1 implementation

2016-02-18 Thread Roger Riggs
Hi Michael, A few comments, nothing severe. Several properties are used in the implementation; is it significant that some are sun.net and others java.net? For the new package can we get away from using the "sun." prefix? Exchange.java: 89 in cancel(), exchImpl may be null since it is not

Re: [9] RFR: 8060097: sun/net/idn/TestStringPrep.java failed.

2016-03-21 Thread Roger Riggs
Hi Naoto, Look fine. Roger On 3/21/2016 11:19 AM, Naoto Sato wrote: Pinging. Can anyone please review this simple fix? Naoto On 3/18/16 10:53 AM, Naoto Sato wrote: Hello, Please review the fix to the following bug: https://bugs.openjdk.java.net/browse/JDK-8060097 The proposed fix is loc

Re: RFR 7167293:FtpURLConnection connection leak on FileNotFoundException

2016-03-23 Thread Roger Riggs
Hi Vyom, I think it should be the case that the same exception (FrpProtocol, IOException) should be rethrown instead of the exception that resulted from the call to close. In many case, the exceptions from a call to a cleanup close() are simply ignored but the alternative is to add them as su

Re: RFR 7167293:FtpURLConnection connection leak on FileNotFoundException

2016-03-29 Thread Roger Riggs
/2016 9:03 AM, vyom wrote: please find that updated webrev(http://cr.openjdk.java.net/~vtewari/7167293/webrev0.3 <http://cr.openjdk.java.net/%7Evtewari/7167293/webrev0.3>), I incorporated Roger's comments. Thanks, Vyom On Wednesday 23 March 2016 10:04 PM, Roger Riggs wrote: Hi Vyo

Re: RFR 7167293:FtpURLConnection connection leak on FileNotFoundException

2016-03-29 Thread Roger Riggs
Roger On 3/29/2016 9:03 AM, vyom wrote: please find that updated webrev(http://cr.openjdk.java.net/~vtewari/7167293/webrev0.3 <http://cr.openjdk.java.net/%7Evtewari/7167293/webrev0.3>), I incorporated Roger's comments. Thanks, Vyom On Wednesday 23 March 2016 10:04 PM, Roger Riggs wr

Re: RFR JDK-8087113: Websocket API and implementation

2016-04-05 Thread Roger Riggs
Hi Pavel, Initial comments, bottom up. It would be helpful if the classnames/filenames reflected the participation in the WebSocket implementation to keep them distinct from the HTTP 2.0 implementation in the same directory. For example, Writer, Reader, etc. perhaps a 'Ws' prefix would be su

Re: RFR JDK-8087113: Websocket API and implementation

2016-04-06 Thread Roger Riggs
Hi Pavel, On 4/5/2016 2:27 PM, Pavel Rappo wrote: Hi Roger, thanks for looking into this. On 5 Apr 2016, at 17:37, Roger Riggs wrote: It would be helpful if the classnames/filenames reflected the participation in the WebSocket implementation to keep them distinct from the HTTP 2.0

Re: RFR JDK-8153353: HPACK implementation

2016-04-11 Thread Roger Riggs
Hi Pavel, Though this is an implementation only package, it could use a few more comments to support maintainers. In the package.html, a couple of hints about what classes are the external interface would be useful and an example of use in Encode and/or Decode would help a lot. The tests u

Re: RFR JDK-8153353: HPACK implementation

2016-04-12 Thread Roger Riggs
after the initial push? On 11 Apr 2016, at 16:18, Roger Riggs wrote: Hi Pavel, Though this is an implementation only package, it could use a few more comments to support maintainers. In the package.html, a couple of hints about what classes are the external interface would be useful and an

Re: RFR: 8155928: Remove hardcoded port numbers from httpclient/Security.java test

2016-05-03 Thread Roger Riggs
Hi Michael, test/java/net/httpclient/security/15.policy: line 15: - Should this policy file include the substitution for the ${port.number}? Otherwise, it still looks like it has a fixed port #. Roger On 5/3/2016 7:44 AM, Michael McMahon wrote: Some tests with hardcoded port numbers were

Re: RFR: 8155928: Remove hardcoded port numbers from httpclient/Security.java test

2016-05-04 Thread Roger Riggs
rity.java) 3) Fixed another strange issue in the Security test which causes failures if the jtreg work directory is not empty prior to running the test. (change in Security.moveFile) Thanks, Michael On 03/05/16 15:47, Roger Riggs wrote: Hi Michael, test/java/net/httpclient/security/15.policy:

Re: RFR: 8153572: [JEP 110] IOException (connection closed for reading) is thrown when try to connect HTTPS service

2016-05-05 Thread Roger Riggs
Hi Michael, Looks fine. Editorial: test/java/net/httpclient/http2/java.httpclient/java/net/http/Http2TestExchange.java Line 8: wildcard import unnecdssary when only using SSLSession test/java/net/httpclient/http2/java.httpclient/java/net/http/Http2TestServerConnection.java Line 34: wildc

Re: RFR: 8155888: java/net/httpclient/QuickResponses.java intermittently failed with java.util.ConcurrentModificationException

2016-05-06 Thread Roger Riggs
+1 On 5/6/2016 1:04 PM, Chris Hegarty wrote: On 5 May 2016, at 12:29, Michael McMahon wrote: Another occasional test case failure. It's a concurrent modification exception caused by modifying a list during processing of the list (by the same thread). The solution is to keep separate lists

Re: RFR 8156825: java/net/httpclient/BasicWebSocketAPITest.java failed with "java.lang.AssertionError"

2016-05-16 Thread Roger Riggs
Hi Michael, +1 HttpClientImpl.java: Unless a large number of classes are needed, individual imports are preferred. (Maybe the IDE did that for you, in which the IDE should be reconfigured.) Roger On 5/16/2016 7:07 AM, Michael McMahon wrote: Could I get the following change reviewed please?

Re: RFR 8156825: java/net/httpclient/BasicWebSocketAPITest.java failed with "java.lang.AssertionError"

2016-05-16 Thread Roger Riggs
constructors. YMMYV Roger On 5/16/2016 10:09 AM, Michael McMahon wrote: Thanks Roger. What would the consensus be on what 'a large number' is? - Michael On 16/05/16 15:11, Roger Riggs wrote: Hi Michael, +1 HttpClientImpl.java: Unless a large number of classes are needed, individu

Re: RFR: 8153142: Cannot resolve multiple values from one response header

2016-05-18 Thread Roger Riggs
Hi Michael, Looks fine. Roger On 5/17/2016 9:43 AM, Michael McMahon wrote: Could I get the following change reviewed please? In some circumstances multi-valued response headers were being ignored (except for the first occurrence). The change fixes that problem. It also changes the output of

Re: RFR 8157105: HTTP/2 client hangs in blocking mode if an invalid frame has been received

2016-05-19 Thread Roger Riggs
Hi Michael, Look ok, A one line description of the error it is trying to produce would be a good addition. Either in comment, javadoc or @summary Roger On 5/18/2016 12:55 PM, Michael McMahon wrote: Could I get the following change reviewed please? http://cr.openjdk.java.net/~michaelm/81571

Re: RFR JDK-8156742: Miscellaneous WebSocket API improvements

2016-06-01 Thread Roger Riggs
Hi Pavel, Many good improvements WebSocket: - CloseReason.of; line 1187: the new constructor of(ByteBuffer) is using IllegalArgumentException inappropriately (And other constructors too) Since the data some off the wire; it is not really the callers fault if it is too short or too lo

Re: RFR [9] 8158525: Update a few java/net tests to use the loopback address instead of the host address

2016-06-02 Thread Roger Riggs
Hi Chris, looks fine but a couple of questions/suggestions. CookieManagerTest.java: - the new definition of localHostAddr could probably be moved to a class static and be used instead of the "127.0.0.1" literal (line 97). - Does the use of the IPV4 loopback address work even in the case of

Re: RFR [9] 8158525: Update a few java/net tests to use the loopback address instead of the host address

2016-06-02 Thread Roger Riggs
Hi Chris, Thanks for the updates and explanations; looks fine. Roger On 6/2/2016 3:12 PM, Chris Hegarty wrote: On 2 Jun 2016, at 14:37, Roger Riggs wrote: Hi Chris, looks fine but a couple of questions/suggestions. Thanks for looking at this Roger. CookieManagerTest.java: - the new

Re: RFR: 8158651: ConcurrentModification exceptions in httpclient

2016-06-03 Thread Roger Riggs
Hi Michael, Looks ok as long as the response_cfs list can't be shrunk by a different thread. (cfs_len can be stale). And 577: probably a printf to system.err isn't the ultimate in robustness or debugging. Roger On 6/3/2016 10:40 AM, Michael McMahon wrote: Could I get the following sma

Re: RFR: 8158651: ConcurrentModification exceptions in httpclient

2016-06-03 Thread Roger Riggs
ested changing the LinkedList to an ArrayList when I told him the size is nearly always small (1 or 2 elements). So, I will also change that. Thanks Michael. On 03/06/16 16:06, Roger Riggs wrote: Hi Michael, Looks ok as long as the response_cfs list can't be shrunk by a different thread. (c

Re: RFR JDK-8156650: Simplify Text message support in WebSocket API

2016-06-06 Thread Roger Riggs
Hi Pavel, Removing the Text interface and its complexity is a good simplification. +1 Editorial: WebSocket.java: 378/379 could be improved: "This implementation passes only complete UTF-16 sequences to the {@code onText} method." Roger On 6/2/2016 2:07 PM, Pavel Rappo wrote: Hi, Could

Re: RFR JDK-8156693: Improve usability of CompletableFuture use in WebSocket API

2016-06-07 Thread Roger Riggs
Hi Pavel, Looks fine. WebSocket.java: - line 82: I'd replace 'never' with 'does not'; it is more like a statement for fact and less a guarantee. - LIne 54: The notation "{@code .onX}" seems unnecessary, is quite cryptic, and can be removed. Are there any tests that would be updated to g

Re: RFR JDK-8156693: Improve usability of CompletableFuture use in WebSocket API

2016-06-08 Thread Roger Riggs
+1, Thanks, Roger On 6/8/2016 9:13 AM, Pavel Rappo wrote: On 7 Jun 2016, at 18:42, Roger Riggs wrote: Hi Pavel, Looks fine. WebSocket.java: - line 82: I'd replace 'never' with 'does not'; it is more like a statement for fact and less a guarantee. - LIne 54:

Re: RFR 8144008: Setting NO_PROXY on an URLConnection is not complied with

2016-06-16 Thread Roger Riggs
Hi Vyom, Looks ok, Roger On 6/16/2016 10:35 AM, Vyom Tewari wrote: Hi All, Please find the latest webrev(http://cr.openjdk.java.net/~vtewari/8144008/webrev0.1/index.html ), i got some off line comments from Chris. Thanks

Re: RFR 8071660 :URLPermission not handling empty method lists correctly

2016-06-17 Thread Roger Riggs
Hi Vyom, URLPermissionTest.java: line 125: it looks odd to assign the same variable twice with the same value. Perhaps it should have been assigning this.url2 = url2. line 330-332: please add a space after "," in argument lists. Roger On 6/17/2016 8:42 AM, Daniel Fuchs wrote: On 17/

Re: RFR 8071660 :URLPermission not handling empty method lists correctly

2016-06-17 Thread Roger Riggs
along with spaces issue. Thanks Vyom. Looks good. -Chris. Thanks, Vyom On Friday 17 June 2016 06:34 PM, Roger Riggs wrote: Hi Vyom, URLPermissionTest.java: line 125: it looks odd to assign the same variable twice with the same value. Perhaps it should have been assigning this.url2

Re: RFR JDK-8156742: Miscellaneous WebSocket API improvements

2016-06-21 Thread Roger Riggs
+1 with Chris' suggested rewording. Roger On 6/21/2016 7:45 AM, Chris Hegarty wrote: On 21 Jun 2016, at 12:21, Pavel Rappo wrote: Hi, Let me try again to propose the following set of changes: http://cr.openjdk.java.net/~prappo/8156742/webrev.02/ This mainly looks fine, just a small comme

Re: Preliminary RFR JDK-8159053: Improve onPing/onClose behavior

2016-06-22 Thread Roger Riggs
Hi Pavel, in general, this treatment is fine, some comments below, On 6/22/2016 8:39 AM, Pavel Rappo wrote: Hi, Here's the updated wording. Could you please have a look at it once again so we fully agree on it? What has changed? 1. "WebSocket implementation"/"WebSocket connection" -> {@code

Re: Preliminary RFR JDK-8159053: Improve onPing/onClose behavior

2016-06-23 Thread Roger Riggs
Hi Pavel, I would avoid terms like application and client in any specification or API description, it may be that the caller of the API is another library. The javadoc should focus on the behavior of each method when it is called. $.02, Roger On 6/23/2016 10:09 AM, Pavel Rappo wrote: Simon

Re: RFR 8158023: SocketExceptions contain too little information sometimes

2016-06-29 Thread Roger Riggs
Yep, Thanks, Roger Thanks Christoph -Original Message- From: nio-dev [mailto:nio-dev-boun...@openjdk.java.net] On Behalf Of Roger Riggs Sent: Mittwoch, 29. Juni 2016 20:20 To: nio-...@openjdk.java.net Subject: Re: RFR 8158023: SocketExceptions contain too little information sometime

Re: RFR JDK-8161091 Incorrect Stream.FlowControl implementation allows to send DataFrame even when window size was exhausted

2016-07-14 Thread Roger Riggs
+1, The revised fix looks correct. Roger On 7/11/2016 6:47 PM, Sergey Kuksenko wrote: I am awfully sorry, previous fix was incorrect. Please, review the right version: http://cr.openjdk.java.net/~skuksenko/jep110/8161091/webrev.01/ On 07/08/2016 02:40 PM, Sergey Kuksenko wrote: Hi, Could

Re: RFR 8163149/9, Typo in java.net.http.AuthenticationFilter

2016-08-05 Thread Roger Riggs
+1 Roger On 8/5/2016 12:05 PM, Felix Yang wrote: Hi there, please review a simple patch for java.net.http.AuthenticationFilter. There is a typo in the code. "Proxy-Authentication" should be replaced with "Proxy-Authenticate" Bug: https://bugs.openjdk.java.net/browse/JDK-8163149 Webrev

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

2016-09-09 Thread Roger Riggs
Hi Rob, Looks ok. Its also a good practice to cleanup the file. (File.deleteOnExit). Roger On 9/9/2016 9:23 AM, Rob McKenna wrote: Hey Chris, Apologies, I'm guilty of "just doing what adjacent tests do" here. That shell script is actually there in the test source already, but generating

Re: RFR: 8157965 update httpserver logging to use java.lang.System.Logger

2016-10-19 Thread Roger Riggs
Hi Daniel, looks fine. (The style differences are disconcerting but are consistent within the file). Roger On 10/19/2016 10:52 AM, Daniel Fuchs wrote: Hi, Please find below a patch that updates jdk.httpserver to use System.Logger. As a result jdk.httpserver no longer requires java.loggin

Re: Temporarily remove java/net/httpclient from jdk_net test group

2016-10-25 Thread Roger Riggs
Hi Chris, Makes sense. Looks fine. Roger On 10/25/2016 8:55 AM, Chris Hegarty wrote: Hi, There are some intermittent test failures in java/net/httpclient. Since development is ongoing over in the sandbox [1], then it is probably best to exclude these tests from the jdk_net test group, in t

Re: RFR(s): 8169002: [TESTBUG] Several java/net/httpclient have undeclared dependency on java.logging module

2016-11-01 Thread Roger Riggs
Hi Sergei, I think it would be preferable to convert the tests to use System.getLogger. Is that possible? Thanks, Roger On 11/1/2016 1:15 PM, Sergei Kovalev wrote: Hello all, Please review a small fix for tests. BugID: https://bugs.openjdk.java.net/browse/JDK-8169002 WebRev: http://cr.openj

Re: RFR(s): 8169002: [TESTBUG] Several java/net/httpclient have undeclared dependency on java.logging module

2016-11-01 Thread Roger Riggs
opped or configured via the java.util.logging.config.file system property if/when needed. $.02, Roger On 11/1/2016 1:53 PM, Daniel Fuchs wrote: Hi Roger, On 01/11/16 17:21, Roger Riggs wrote: Hi Sergei, I think it would be preferable to convert the tests to use System.getLogger. Is that pos

Re: RFR(s): 8169002: [TESTBUG] Several java/net/httpclient have undeclared dependency on java.logging module

2016-11-01 Thread Roger Riggs
On 11/1/2016 2:34 PM, Daniel Fuchs wrote: Hi Roger, I think we agree :-) On 01/11/16 18:01, Roger Riggs wrote: Hi Daniel, It seemed useful to be able to run the test in as many environments as possible though realistically java.util.logging may be there too. I don't see that setting t

Re: RFR: 8167178 Exported elements referring to inaccessible types in java.naming

2017-01-17 Thread Roger Riggs
Hi Vyom, Please also correct or remove the comment saying it should be treated as read-only by subclasses. Roger On 1/16/2017 4:56 AM, Chris Hegarty wrote: Looks good. Thanks Vyom. -Chris. On 16 Jan 2017, at 09:10, Vyom Tewari wrote: Hi All, Please review below the small fix for the

Re: RFR: 8167178 Exported elements referring to inaccessible types in java.naming

2017-01-18 Thread Roger Riggs
NameImpl impl; /** * Syntax properties for this compound name. * This field is initialized by the constructors and cannot be null. On Tuesday 17 January 2017 09:20 PM, Roger Riggs wrote: Hi Vyom, Please also correct or remove the comment saying it should be treated as read-only

Re: RFR (xs) : 8177144 :sun/net/www/http/HttpClient/B8025710.java should run in ovm mode

2017-03-20 Thread Roger Riggs
Hi Sean, Looks good, Roger On 3/20/2017 12:11 PM, Seán Coffey wrote: Looking for a simple review. Test should be run in ovm mode : https://bugs.openjdk.java.net/browse/JDK-8177144 diff --git a/test/sun/net/www/http/HttpClient/B8025710.java b/test/sun/net/www/http/HttpClient/B8025710.java -

Re: JDK10 RFR: 8165437 Evaluate the use of gettimeofday in Networking code

2017-04-12 Thread Roger Riggs
Hi Vyom, Thanks for taking this on. I have a few comments and questions. In aix_close.c line 547 The code for if (nanoTimeout >= NSEC_PER_MSEC) seems ineffective. The update of nanoTime at 549-550 ensures the timeout is > NSEC_PER_MSEC if it loops. On the first pass through the code if nan

Re: RFR [9] 8177536: Avoid Apple Peer-to-Peer interfaces in networking tests

2017-04-12 Thread Roger Riggs
Hi Chris, Looks good, nice use of streams. B6558853.java: 28: an extra "*" A few tests do nothing unless an interface is found; they could use either output that says what interface is tested or a message that no interface was available. (In case someone is reviewing the logs and wants to kn

Re: RFR (10) : 8177457 and 8177452

2017-04-19 Thread Roger Riggs
Hi Michael, Looks fine. But they are applicable to JDK 9 and as doc changes are acceptable. Why JDK 10? Roger On 4/19/2017 12:36 PM, Michael McMahon wrote: Could I get the following JDK 10 reviews please. They are minor doc cleanups, that do not involve spec changes https://bugs.openjdk.jav

Re: RFR 8166139/10, Refactor java/net shell cases to java

2017-05-25 Thread Roger Riggs
Hi Felix, closetest/CloseTest: 32: Please put the @modules directives together (and not repeat) 44: do not use wildcard imports (reconfigure your IDE if necessary to avoid accidents). 63: setup() is invoked twice... remove 1 or explain why 2 calls 69, 78,90,91,... : space in method call

Re: RFR 8166139/10, Refactor java/net shell cases to java

2017-05-26 Thread Roger Riggs
/GetResourceAsStream.java URLConnection/6212146/Test.java:47 typo: ULR -> URL Regards, Roger On 5/25/2017 11:14 PM, Felix Yang wrote: Hi Roger, please review the updated webrev: http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/ Thanks, Felix On 2017/5/26 3:24, Roger Riggs wrote: Hi Fe

Re: RFR 8182975/10, Mark http2/BasicTest.java and FixedThreadPoolTest.java as intermittently failing

2017-06-29 Thread Roger Riggs
Looks fine Felix, Roger On 6/29/2017 3:59 AM, Felix Yang wrote: Hi all, please review a minor patch to add key intermittent to two tests. They have been observed to be failing intermittently. Bug: https://bugs.openjdk.java.net/browse/JDK-8182975 Patch: diff -r 7cef1c3f1dbe test/j

Re: [JDK 10] RFR: 8185794: java/net/httpclient/security/Driver.java fails in timeout

2017-08-03 Thread Roger Riggs
Hi Daniel, Looks fine, Roger On 8/3/2017 2:21 PM, Daniel Fuchs wrote: Hi, Please find below a fix for a test bug: https://bugs.openjdk.java.net/browse/JDK-8185794 8185794: java/net/httpclient/security/Driver.java fails in timeout http://cr.openjdk.java.net/~dfuchs/webrev_8185794/webrev.00/

Re: RFR 8186818/10, Enable debug option for TcpTest.java

2017-08-28 Thread Roger Riggs
Hi Felix, Looks fine. Roger On 8/28/2017 3:51 AM, Felix Yang wrote: Hi there,     please review the patch to enable debug diagnostic for test/java/net/ipv6tests/TcpTest.java. Then we can know which addresses are being used, to isolate possible multi-homed issues. Bug:     https://bugs.op

Re: RFR 8134989/10, java/net/MulticastSocket/TestInterfaces.java failed due to unexpected IP address

2017-09-01 Thread Roger Riggs
Hi Felix, Looks ok; though could be simpler to just print all the addresses of all NIs. findIntefacesWithDupAddress could use streams more effectively (if its worth the time to rewrite). The inetAddresses() method on NetworkInterface produces a stream of InetAddress which could be filtere

Re: RFR[10]: 8185072 network006 times out in many configs in JDK10-hs nightly

2017-09-25 Thread Roger Riggs
Hi Vyom, Looks fine to me too. My only concern was adding an intensive test (used to be a stress test) to the normal test cycles.  I'd be more comfortable with it being in a separate/lower tier or as @manual. Roger On 9/24/2017 1:15 PM, Chris Hegarty wrote: On 15 Sep 2017, at 10:29, vyom

Re: RFR[10]: 8187658: Bigger buffer for GetAdaptersAddresses

2017-09-25 Thread Roger Riggs
Hi Ivan, Looks ok to me. I don't see a reason to skimp on the additional size since it is allocated and then freed immediately. The increment might as well be as big as the initial size. I don't see a reason to retry if the buffer gets close to ULONG_MAX; I'd just break the for loop and let

Re: RFR[10]: 8187658: Bigger buffer for GetAdaptersAddresses

2017-09-25 Thread Roger Riggs
Hi Ivan, Looks fine, Thanks, Roger On 9/25/2017 4:58 PM, Ivan Gerasimov wrote: Thank you Roger for review! On 9/25/17 11:47 AM, Roger Riggs wrote: Hi Ivan, Looks ok to me. I don't see a reason to skimp on the additional size since it is allocated and then freed immediately.

Re: RFR 8145635 : Add TCP_QUICKACK socket option

2017-10-16 Thread Roger Riggs
Hi Vyom, A few suggestions: PlainDatagramSocketImpl.java:  - line 95/96:  I think you can use just forEach, the order version is not necessary.     The code will be a bit more readable if the .filter and .forEach are on a new line and don't wrap.     You can also remove the extra "(" and ")

Re: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

2017-10-17 Thread Roger Riggs
Hi, Keep the synchronized, it will be low overhead since the Vector operations are synchronized and in the same thread. I think a CCE could occur during the iteration to find the entry when Vector.Itr.next() checks. (It you want to more radical fix, replace the Vector with something more cur

Re: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

2017-10-18 Thread Roger Riggs
lto:net-dev-boun...@openjdk.java.net] *On Behalf Of *Roger Riggs *Sent:* Dienstag, 17. Oktober 2017 16:53 *To:* net-dev@openjdk.java.net *Subject:* Re: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache Hi, Keep the synchronized, it will be low overhead since

Re: RFR(S): 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

2017-10-18 Thread Roger Riggs
super.remove()) then the next call to the Iterator would throw a CME. But we don’t go back to the iterator as we return after removing. Nevertheless, I can still remove the comment or change it… let me know. Thanks Christoph *From:*Roger Riggs [mailto:roger.ri...@oracle.com] *Sent:* Mittwoch, 18

Re: [jdk10] (XXS) RFR 8189631 : Missing space in the javadoc for InetAddress.createNameService()

2017-11-14 Thread Roger Riggs
+1, Reviewed Roger On 11/14/2017 2:45 PM, Ivan Gerasimov wrote: Hello! A typo in the javadoc: --- a/src/java.base/share/classes/java/net/InetAddress.java +++ b/src/java.base/share/classes/java/net/InetAddress.java @@ -1133,7 +1133,7 @@ /**   * Create an instance of the NameService i

Re: RFR:8190843 can not set/get extendedOptions to ServerSocket

2017-12-01 Thread Roger Riggs
Hi Vyom, Looks fine Roger On 11/16/2017 4:03 AM, vyom tewari wrote: Hi All, Please review the small code change for the below issue. Webrev : http://cr.openjdk.java.net/~vtewari/8190843/webrev0.0/index.html BugId    : https://bugs.openjdk.java.net/browse/JDK-8190843 In this cod

  1   2   >