[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8153191: UL: Separator in VM.log output is redundant
From:       Yasumasa Suenaga <yasuenag () gmail ! com>
Date:       2016-04-26 9:27:30
Message-ID: 951bae53-6f6c-ab0d-602d-2aadc067fcf4 () gmail ! com
[Download RAW message or body]

Hi Marcus,

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

Oh, thanks!
I will merge Dmitry's suggestion in this change request.


Thanks,

Yasumasa


On 2016/04/26 17:39, Marcus Larsson wrote:
>
>
> 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
>>>>>>
>>>>>>
>>>>>
>>>
>>>
>
[prev in list] [next in list] [prev in thread] [next in thread]