RFR: 8157948: UL allows same log file with multiple file=
david.holmes at oracle.com
Mon Aug 29 01:45:05 UTC 2016
On 26/08/2016 10:11 PM, Marcus Larsson wrote:
> Hi David,
> Thanks for looking at this!
> New webrev:
Why bother with implicit_output_prefix instead of using
LogFileOutput::Prefix directly? You do use the latter in two places so I
find the inconsistency strange.
> See replies below.
Follow up 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:
>>> 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
>> 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.
>> 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,
> Fixed, see below.
>> 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.
Placement is fine.
>> 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.
Yes you are right it was stripping them away - it is an error.
>>> New unit test through JPRT
More information about the hotspot-runtime-dev