RFR: JDK-8153191: UL: Separator in VM.log output is redundant

[Yasumasa Suenaga]

Thanks Dmitry,

If this change is accepted, I will fix as below:
---------
char delimiter = 0;

for (...) {
     printf("%c%s", delimiter, LogDecorators::name(decorator));
     delimiter = ',';
}
---------


Thanks,

Yasumasa


On 2016/04/25 21:44, Dmitry Samersoff wrote:
> Yasumasa,
>
> It might be better to deal with first comma as:
>
> char delimiter[2] = {0,0};
>
> for (...) {
>     printf("%s%s", delimiter, LogDecorators::name(decorator));
>     *delimiter = ',';
> }
>
> -Dmitry
>
> On 2016-04-25 15:19, Yasumasa Suenaga wrote:
>> Hi Marcus,
>>
>>> Please see
>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-March/019237.html
>>>
>>> for the configuration string issue. It was sent out earlier today.
>>
>> Did you fix for trailing comma from decoration list?
>> Comma for decoration exists yet in result of VM.log list jcmd.
>>
>>> Your suggested fix for the decorator listing is broken. The for loop you
>>> modified iterates over all possible decorators, not all enabled
>>> decorators. So unless you have the last possible decorator enabled you
>>> will still get a trailing comma.
>>
>> I think we can fix as below.
>> Should I file it to JBS as new issue?
>> -------------------
>> diff -r 3d289e4ba366 src/share/vm/logging/logConfiguration.cpp
>> --- a/src/share/vm/logging/logConfiguration.cpp Fri Apr 22 19:40:39 2016
>> +0200
>> +++ b/src/share/vm/logging/logConfiguration.cpp Mon Apr 25 21:14:49 2016
>> +0900
>> @@ -408,10 +408,13 @@
>>    out->print_cr("Log output configuration:");
>>    for (size_t i = 0; i < _n_outputs; i++) {
>>      out->print("#" SIZE_FORMAT ": %s %s ", i, _outputs[i]->name(),
>> _outputs[i]->config_string());
>> +    bool first_decorator = true;
>
>>      for (size_t d = 0; d < LogDecorators::Count; d++) {
>>        LogDecorators::Decorator decorator =
>> static_cast<LogDecorators::Decorator>(d);
>>        if (_outputs[i]->decorators().is_decorator(decorator)) {
>> -        out->print("%s,", LogDecorators::name(decorator));
>> +        out->print("%s%s", first_decorator ? "" : ",",
>> +                           LogDecorators::name(decorator));
>> +        first_decorator = false;
>>        }
>>      }
>>      out->cr();
>> -------------------
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2016/03/31 22:11, Marcus Larsson wrote:
>>> Hi,
>>>
>>> On 03/31/2016 02:50 PM, Yasumasa Suenaga wrote:
>>>> Hi all,
>>>>
>>>> We can check log configuration through VM.log list jcmd:
>>>> -------
>>>> Log output configuration:
>>>> #0: stdout vmoperation=debug,safepoint=info, uptime,level,tags,
>>>> #1: stderr all=warning uptime,level,tags,
>>>> #2: gc.log gc=trace, uptime,level,tags,
>>>> -------
>>>>
>>>> Configuration string and decorators are ended with comma.
>>>> We should remove it as below:
>>>> -------
>>>> Log output configuration:
>>>> #0: stdout vmoperation=debug,safepoint=info uptime,level,tags
>>>> #1: stderr all=warning uptime,level,tags
>>>> #2: gc.log gc=trace uptime,level,tags
>>>> -------
>>>>
>>>>
>>>> I uploaded webrev for this issue.
>>>> Could you review it?
>>>>
>>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8153191/webrev.00/
>>>
>>> Please see
>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-March/019237.html
>>>
>>> for the configuration string issue. It was sent out earlier today.
>>>
>>> Your suggested fix for the decorator listing is broken. The for loop you
>>> modified iterates over all possible decorators, not all enabled
>>> decorators. So unless you have the last possible decorator enabled you
>>> will still get a trailing comma.
>>>
>>> Also, please note that UL belongs to the serviceability subcomponent,
>>> and patches like this should probably go to the serviceability-dev
>>> mailing list.
>>>
>>> Thanks,
>>> Marcus
>>>
>>>>
>>>>
>>>> I cannot access JPRT.
>>>> So I need a sponsor.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>
>
>