Re: RFR 8151281: Module java.httpclient could use System.Logger instead of PlatformLogger

2016-03-15 Thread Daniel Fuchs 

On 15/03/16 06:35, Mandy Chung wrote:

It’s a nice trick.  Perhaps define a new internal class that implements 
System.Logger with default methods.  Any platform classes want itself to be 
skipped will extend that new internal class?


I thought the purpose here was to avoid using
internal stuff requiring qualified exports.

It would have been easy to implement System.Logger anyway,
since most of its methods have a default body.

best regards,

-- daniel

RFR 8151586: Wrong exception catch for FTPClient in JDK-8055032

2016-03-15 Thread vyom 

Hi,
Please review the below fix.

Bug: JDK-8151586 : Wrong exception catch for FTPClient in JDK-8055032
Webrev  :http://cr.openjdk.java.net/~rgoel/~vyom/8151586/webrev0.0/ 



Thanks,
Vyom

RFR 7167293:FtpURLConnection connection leak on FileNotFoundException

2016-03-15 Thread vyom 

Hi,

Please review the below fix.

Bug: JDK-7167293 : FtpURLConnection connection leak on FileNotFoundException
Webrev: http://cr.openjdk.java.net/~rgoel/~vyom/7167293/webrev0.0/ 



Thanks,
Vyom

Re: RFR 8151586: Wrong exception catch for FTPClient in JDK-8055032

2016-03-15 Thread Chris Hegarty 

Vyom,

On 15/03/16 09:51, vyom wrote:

Hi,
Please review the below fix.

Bug: JDK-8151586 : Wrong exception catch for FTPClient in JDK-8055032
Webrev  :http://cr.openjdk.java.net/~rgoel/~vyom/8151586/webrev0.0/



The source change looks ok.

The test attempts to connect to an external ftp site. It
just fails on my machine with java.net.NoRouteToHostException.

Does the test need to run in othervm mode. I think not.

To simplify the test, just add 'throws IOException' to
main, so you don't have to deal with wrapping these in
RuntimeException. If a test fails with an IOException it
will be reported as failed by the test harness. Try it.

-Chris.

Re: RFR 7167293:FtpURLConnection connection leak on FileNotFoundException

2016-03-15 Thread Chris Hegarty 

Vyom,

On 15/03/16 10:00, vyom wrote:

Hi,

Please review the below fix.

Bug: JDK-7167293 : FtpURLConnection connection leak on
FileNotFoundException
Webrev: http://cr.openjdk.java.net/~rgoel/~vyom/7167293/webrev0.0/



You have the same lines of code in a number of places. Does
a private static helper method make sense for this?

Test tries to connect to an external resource, which is not
reachable on my, and many, systems.  Can the test setup a simple
ServerSocket to do something minimal?

-Chris.