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

[Rachel Protacio]

Thanks, Coleen :) Fixed as requested.
Rachel

On 3/25/2016 12:15 PM, Coleen Phillimore wrote:
>
>
> On 3/25/16 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.
>
> Yes, this is a really good code review.   Just to make it a bit 
> shorter, you could declare the constant max_tagset_len = 
> max_tagset_size - 1.
>
> Just to prove I really looked at it, there is also a space that should 
> be removed here:
>
> 978 void log_deprecated_flag( const char* name, const char* pref, 
> AliasedLoggingFlag alf) {
>
>
> I don't need to see it again for that though.
> Thanks,
> Coleen
>
>>
>> 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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>