RFR 8215398: -Xlog option usage => Invalid decorator '\temp\app_cds.log'.

[serguei.spitsyn at oracle.com]

Hi Harold,

It looks good to me.

Thanks,
Serguei

On 12/20/18 1:30 PM, Harold David Seigel wrote:
> Hi David,
>
> Thanks for looking at this!
>
> Please review this updated webrev.  The fix is the same but the webrev 
> contains a new test instead of modifying an existing test:
>
>    http://cr.openjdk.java.net/~hseigel/bug_8215398.2/webrev/
>
> The logging implementation does not handle single quotes as expected.  
> For example, if the TestQuotedLogOutputs.java test is changed to use 
> single quotes instead of double quotes, it will fail, even on Linux, 
> because the single quotes are included as part of the file name.  They 
> are not stripped off.  That is why "java 
> -Xlog:safepoint=trace:'d:\safepointtrace.txt' -version" fails.
>
> Perhaps a new bug is needed to change logging's handling of single 
> quotes?  It's a bit surprising that this issue hasn't already been 
> reported.
>
> Thanks, Harold
>
> On 12/20/2018 2:07 AM, David Holmes wrote:
>> Hi Harold,
>>
>> On 19/12/2018 11:22 pm, Harold David Seigel wrote:
>>> Hi,
>>>
>>> Please review this small change to fix JDK-8215398.  The fix works 
>>> by not treating ':' as a delimiter in a -Xlog... option string if it 
>>> is following by a '\' and preceded by either a single character or 
>>> the text 'file='.  The fix is for Windows only.
>>>
>>> Open Webrev: 
>>> http://cr.openjdk.java.net/~hseigel/bug_8215398/webrev/index.html
>>
>> I think I can follow the fix.
>>
>> But I'm a bit concerned about the test. AFAICS the test thought it 
>> was already testing this case - albeit with the path quoted:
>>
>>   42         // Ensure log files can be specified with full path.
>>   43         // On windows, this means that the file name will contain
>>   44         // a colon ('C:\log.txt' for example), which is used to
>>   45         // separate -Xlog: options 
>> (-Xlog:tags:filename:decorators).
>>   46         // Try to log to a file in our current directory, using 
>> its absolute path.
>>   47         String baseName = "test file.log";
>>   48         Path filePath = Paths.get(baseName).toAbsolutePath();
>>   49         String fileName = filePath.toString();
>>   50         File file = filePath.toFile();
>> ...
>>   65         String[] validOutputs = new String[] {
>>   66             quote + fileName + quote,
>>   67             "file=" + quote + fileName + quote,
>>   68             quote + fileName + quote + ":",
>>   69             quote + fileName + quote + "::"
>>   70         };
>>
>> But even quoted if I specify the drive designator in the path, it 
>> fails! Here's a local attempt at this:
>>
>> D:\ade> apps\Java\jdk-11\fastdebug\bin\java 
>> -Xlog:safepoint=trace:'d:\safepointtrace.txt' -version
>> [0.014s][error][logging] Invalid decorator '\safepointtrace.txt''.
>> Invalid -Xlog option '-Xlog:safepoint=trace:'d:\safepointtrace.txt'', 
>> see error log for details.
>> Error: Could not create the Java Virtual Machine.
>> Error: A fatal exception has occurred. Program will exit.
>>
>> So AFAICS the test can't possibly have been testing what it thought 
>> it was testing! ???
>>
>> I'm also not sure about just adding some unquoted variants to the 
>> existing TestQuotedLogOutputs without renaming the test and updating 
>> the @summary
>>
>> Thanks,
>> David
>>
>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8215398
>>>
>>> The fix was regression tested by running Mach5 tiers 1 and 2 tests 
>>> and builds on Linux-x64, Windows, and Mac OS X, running tiers 3-5 
>>> tests on Linux-x64, running JCK-12 Lang and VM tests on Linux-x64, 
>>> and by hand on Windows.
>>>
>>> Thanks, Harol
>>>
>