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

[Dmitry Dmitriev]

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