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

[Marcus Larsson]

On 2016-04-25 14:54, Yasumasa Suenaga wrote:
> Thanks Dmitry,
>
> If this change is accepted, I will fix as below:
> ---------
> char delimiter = 0;
>
> for (...) {
>     printf("%c%s", delimiter, LogDecorators::name(decorator));
>     delimiter = ',';
> }
> ---------

If you make it a char then the string would be null terminated right in 
the beginning.

Marcus

>
>
> 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
>>>>>
>>>>>
>>>>
>>
>>