RFR: 8145235: Deprecate product flags that have been converted to Unified Logging

[Rachel Protacio]

Will do. Thanks!
Rachel

On 3/25/2016 12:09 PM, Dmitry Dmitriev wrote:
> Rachel,
>
> Thank you for fixing that! One missing change: 'max_tagset_size - 
> strlen(tagset_buffer) - 1' must be instead of '1' in strncat on line 
> 989. Not need a new webrev for that.
>
> Dmitry
>
> On 25.03.2016 19:01, Rachel Protacio wrote:
>> Hi, Dmitry,
>>
>> Wow, thank you for catching all of those! You were right, they were 
>> oversights on my part. I have fixed accordingly: 
>> http://cr.openjdk.java.net/~rprotacio/8145235.03/
>>
>> Rachel
>>
>> On 3/25/2016 10:55 AM, Dmitry Dmitriev wrote:
>>> Hi Rachel,
>>>
>>> Few comments/questions about arguments.cpp:
>>> 1) I see that AliasedLoggingFlag contains six LogTagType from tag0 
>>> to tag5, but 'LogTagType tagSet' contains only up to 'tag4'. Is it 
>>> correct?
>>>
>>> 2) I think that you can declare tagSet without specifying explicit 
>>> size, i.e. ' LogTagType tagSet[] = {...}'. This can help in future 
>>> if size of the arrays is increased. In this case the number of 
>>> elements will be 'sizeof(tagset)/sizeof(tagset[0])', i.e. you can 
>>> change 'i < 5' to 'i < sizeof(tagset)/sizeof(tagset[0])'. Fell free 
>>> to implement this suggestion or not!
>>>
>>> 3) I think that third argument to strncat on line 990 should be 
>>> 'max_tagset_size - strlen(tagset_buffer) - 1' to have space for 
>>> termination '0' byte.
>>>
>>> 4) Also, I think that  'max_tagset_size - strlen(tagset_buffer) - 1' 
>>> must be instead of '1' in strncat on line 988.
>>>
>>> 5) I think that instead of 'memset(tagset_buffer, 0, 
>>> max_tagset_size);' on line 983 you can just write tagset_buffer[0] = 
>>> '\0';
>>>
>>> Thank you,
>>> Dmitry
>>>
>>> On 24.03.2016 23:31, Rachel Protacio wrote:
>>>> Hi,
>>>>
>>>> Thanks for pointing that out. I realized I could save some code by 
>>>> using strncat(), so I did that instead.
>>>> http://cr.openjdk.java.net/~rprotacio/8145235.02/
>>>>
>>>> Thanks,
>>>> Rachel
>>>>
>>>> On 3/24/2016 3:52 PM, Coleen Phillimore wrote:
>>>>>
>>>>> Hi Rachel,
>>>>>
>>>>> http://cr.openjdk.java.net/~rprotacio/8145235.01/src/share/vm/runtime/arguments.cpp.udiff.html 
>>>>>
>>>>> I think you need strncpy to make sure your tagset buffer doesn't 
>>>>> overflow.
>>>>>
>>>>> Coleen
>>>>>
>>>>> On 3/24/16 3:37 PM, Rachel Protacio wrote:
>>>>>> Thanks for the review! And good point. It's to the 'arguments' 
>>>>>> tag to follow the gc team's logging-deprecation precedent. Here's 
>>>>>> a new webrev updating your non-product error messages to follow 
>>>>>> this scheme as well: 
>>>>>> http://cr.openjdk.java.net/~rprotacio/8145235.01/
>>>>>>
>>>>>> Builds and passes logging/RemovedDevelopFlagsTest.java.
>>>>>>
>>>>>> Rachel
>>>>>>
>>>>>> On 3/24/2016 10:56 AM, Max Ockner wrote:
>>>>>>> Rachel,
>>>>>>> This looks fine. Why are we logging this message to arguments 
>>>>>>> tag? Should we be logging the non-product error messages  to 
>>>>>>> arguments as well?
>>>>>>> Thanks,
>>>>>>> Max
>>>>>>> On 3/22/2016 3:21 PM, Rachel Protacio wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please review this enhancement that prints a deprecation 
>>>>>>>> message for the product flags we have aliased to UL options. We 
>>>>>>>> wanted to mimic the GC deprecation messages as seen in 
>>>>>>>> arguments.cpp, e.g.
>>>>>>>>
>>>>>>>>    log_warning(gc)("-XX:+PrintGCDetails is deprecated. Will use
>>>>>>>>    -Xlog:gc* instead.");
>>>>>>>>
>>>>>>>> I have logged our deprecation messages to a new tag 'arguments' 
>>>>>>>> at the 'warning' level (warning-level logging is always 
>>>>>>>> printed, without having to be directly specified in the command 
>>>>>>>> line).
>>>>>>>>
>>>>>>>> Basic sample output:
>>>>>>>>
>>>>>>>>    $ java -XX:+TraceMonitorInflation -version
>>>>>>>>    [0.001s][warning][arguments] -XX:+TraceMonitorInflation is
>>>>>>>>    deprecated. Will use -Xlog:monitorinflation=debug instead.
>>>>>>>>    ...
>>>>>>>>
>>>>>>>> Sample output with "-"/"off" and multiple tags:
>>>>>>>>
>>>>>>>>    $ java -XX:-TraceClassLoadingPreorder -version
>>>>>>>>    [0.001s][warning][arguments] -XX:-TraceClassLoadingPreorder is
>>>>>>>>    deprecated. Will use -Xlog:classload,preorder=off instead.
>>>>>>>>    ...
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8145235
>>>>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8145235/
>>>>>>>>
>>>>>>>> Tested locally for robustness w.r.t. multi-tag tagsets, 
>>>>>>>> multiple aliased flags, etc.
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Rachel
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>