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

[Dmitry Dmitriev]

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