Re: RFR 8022580: sun.net.ftp.impl.FtpClient.nameList(null)

2016-07-07 Thread Artem Smotrakov
Hi Svetlana, Thank you for addressing the comments below. The test looks fine to me. Just a couple of minor comments. 1. You don't really need fields in lines 77-79. 2. try-catch block in line 86 is not really necessary as well. It would be better to update bug subject to something more mean

Re: RFR 8022580: sun.net.ftp.impl.FtpClient.nameList(null)

2016-07-07 Thread Svetlana Nikandrova
Artem, thank you for suggested test improvements. Here is updated webrev: http://cr.openjdk.java.net/~snikandrova/8022580/webrev.02/ Chris, seem like you have already looked into that issue before. May be you can find some time t

Re: RFR 8022580: sun.net.ftp.impl.FtpClient.nameList(null)

2016-07-06 Thread Artem Smotrakov
Hi Svetlana, Thanks for addressing my comments. Could you please take a look at a couple of comments about TestFtpClientNameListWithNull.java below? 1. lines 64-70: should "client" be closed in "finally" block? I also think it might be better to re-throw IOExceptions instead of ignoring them

Re: RFR 8022580: sun.net.ftp.impl.FtpClient.nameList(null)

2016-07-06 Thread Svetlana Nikandrova
Hi Artem, thank you for your replay. I believe I addressed all your comments. Please see updated diff: http://cr.openjdk.java.net/~snikandrova/8022580/webrev.01/ Thank you, Svetlana On 05.07.2016 20:54, Artem Smotrakov wrote: Hi

Re: RFR 8022580: sun.net.ftp.impl.FtpClient.nameList(null)

2016-07-05 Thread Artem Smotrakov
Hi Svetlana, I am not an official reviewer, but I have a couple comments below. TestFtpClientNameListWithNull.java: 1. Shouldn't "commandHasArgs" be volatile? Looks like it may cause intermittent failures. As a sanity check, you may want to run the test in a loop to make sure it is stable.