RFR: 8231863: Crash if classpath is read from @argument file and the main gets option argument

[Henry Jen]

Thanks Alan and Mandy for the review.

I am guessing the Alan’s preference to use Files.write(aFilePath, lines) is to avoid extra String.join operation, which I would too. The current version with byte[] is more flexible but we most likely won't need it in launcher. Anyway, I don’t think the difference would be noticeable with launcher tests.

The updated webrev[1] takes a boolean to decide which Files.write to use but gives up the byte[] version.

Cheers,
Henry

[1] http://cr.openjdk.java.net/~henryjen/jdk/8231863.1/webrev/

> On Nov 9, 2019, at 11:33 AM, Mandy Chung <mandy.chung at oracle.com> wrote:
> 
> The patch looks fine to me as well.
> 
> As for the test, perhaps adding a new createAFile(File aFile, List<String> lines, boolean hasTrailingBlankLine) in TestHelper instead may help avoid any confusion.
> 
> 
> Mandy
> 
> On 11/8/19 7:43 AM, Mat Carter wrote:
>> Hi Alan
>> 
>> The method you propose: [nio.]Files[.write](aFile.toPath, lines) adds a trailing blank line to the file; the regression test needs to generate a file without a trailing blank line as this is the condition in which the bug occurs.  This is why it now writes out an array of bytes
>> 
>> Cheers
>> Mat
>> 
>> ________________________________
>> From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> on behalf of Alan Bateman <Alan.Bateman at oracle.com>
>> Sent: Friday, November 8, 2019 2:56 AM
>> To: Henry Jen <henry.jen at oracle.com>; core-libs-dev at openjdk.java.net <core-libs-dev at openjdk.java.net>
>> Subject: Re: RFR: 8231863: Crash if classpath is read from @argument file and the main gets option argument
>> 
>> On 07/11/2019 22:55, Henry Jen wrote:
>>> Hi,
>>> 
>>> Please review the webrev[1], contributed by Mat Carter. You can find the bug details at JBS[2]. I have reviewed and tested the fix, I still need an official review before I can push this.
>>> 
>> Looks okay although in the test, the createAFile helper method could be
>> replaced with Files(aFile.toPath, lines) and that would avoid the need
>> to concatenate all the lines. You can specify the defaultCharset to that
>> method if you need really it.
>> 
>> -Alan
>