RFR: 8157948: UL allows same log file with multiple file=

Robbin Ehn robbin.ehn at oracle.com
Thu Sep 1 12:43:11 UTC 2016


Hi Marcus,

On 08/26/2016 02:11 PM, Marcus Larsson wrote:
> Hi David,
>
> Thanks for looking at this!
>
> New webrev:
> http://cr.openjdk.java.net/~mlarsson/8157948/webrev.01/

Looks good, thanks!

/Robbin

>
> Incremental:
> http://cr.openjdk.java.net/~mlarsson/8157948/webrev.00-01/
>
> See replies below.
>
> On 08/26/2016 03:44 AM, David Holmes wrote:
>> Hi Marcus,
>>
>> We really need a better way to specify and verify these mini-grammars for command-line options. :(
>
> Yeah, I'm all for something like that.
>
>>
>> On 25/08/2016 7:31 PM, Marcus Larsson wrote:
>>> Hi,
>>>
>>> Please review the following patch to fix the issue where you could have
>>> the same file added twice as different log outputs in UL if it had the
>>> "file=" prefix or if it was quoted. Log output names are now normalized
>>> during log argument parsing to ensure they are always normalized when
>>> finding existing or adding new outputs.
>>
>> So does this mean that whereas today
>>
>> -Xlog:gc=debug:foo
>>
>> assumes foo is the log file, with this fix you will get an error?
>
> No, the file= prefix will be assumed just like before. The parse step will now explicitly add it in the case that it wasn't specified. So every LogFileOutput instance
> created will have the prefix in its name.
>
>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~mlarsson/8157948/webrev.00/
>>
>> src/share/vm/logging/logFileOutput.cpp
>>
>> Suggestion:
>>
>>    const char* prefix = "file=";
>>    assert(strstr(name, prefix) == name, "invalid output name '%s': missing prefix: %s", name, prefix);
>>    _file_name = make_file_name(name + strlen(prefix), _pid_str, _vm_start_time_str);
>
> Fixed, see below.
>
>>
>> ---
>>
>> src/share/vm/logging/logConfiguration.cpp
>>
>> Suggestion:
>>
>> static const char* prefix = "file=";
>
> I've refactored all "file=" literals into constants, but I made the constant a field of LogFileOutput. I think it fits better there, let me know if you think otherwise.
>
>>
>> In normalize_output_name it is hard for me to work out what the possible "grammar" is, or how different cases will be handled. Currently -Xlog:gc=debug:"file"=foo is
>> treated as -Xlog:gc=debug:file=foo. But with your changes I think the quoting will be handled differently.
>
> Actually -Xlog:gc=debug:"file"=foo should give an error, since quoting the output types isn't supported (only the name can be quoted). This should just be a refactoring to
> make sure we're always managing the output names in a uniform manner (so that file="foo" and file=foo isn't treated as two different log outputs).
>
> BTW, take care if you're testing this on the command line, as the shell might be stripping away quotes in the arguments for you.
>
> Thanks,
> Marcus
>
>>
>> Thanks,
>> David
>>
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8157948
>>>
>>> Testing:
>>> New unit test through JPRT
>>>
>>> Thanks,
>>> Marcus
>


More information about the hotspot-runtime-dev mailing list