RFR (M): 8112746: Followup to JDK-8059557 (JEP 245: Validate JVM Command-Line Flag Arguments)

[Coleen Phillimore]

The improvements look really good to me.

Coleen

On 8/7/15 1:50 PM, gerard ziemski wrote:
>
>
> On 08/07/2015 12:30 PM, sangheon.kim wrote:
>> Hi Gerard,
>>
>> Before looking at this new webrev, I found that below are missing.
>>
>> 'apply_int / apply_uint' from
>> CommandLineFlagConstraintList::check_constraints()
>> 'check_int / check_uint' from CommandLineFlagRangeList::check_ranges()
>>
>> If you want to handle these from other RFE, I'm fine.
>
> Great catch, I would prefer to handle this here now and be done. Fixed.
>
>
> cheers
>
>>
>> Thanks,
>> Sangheon
>>
>>
>> On 08/07/2015 09:36 AM, gerard ziemski wrote:
>>> hi,
>>>
>>> Please review this webrev 3 of the follow-up fixes including:
>>>
>>> - Passing values by value, not pointer, to constraint functions. It
>>> was thought originally that me way want to “fix” the values, but it
>>> was decided to be out of scope for the JEP. (requested by Coleen)
>>>
>>> - Add print_error_as_needed() utility function to range/constraint
>>> code to improve code robustness (requested by Coleen)
>>>
>>> - Remove debug code (requested by Kim Barrett)
>>>
>>> - Implement Flag::flag_error_str in .cpp file (requested by Kim 
>>> Barrett)
>>>
>>> - Only check constraint if range passes (requested by Kim Barrett)
>>>
>>> - Merged with Sangheon Kim fix for "Add additional validation after
>>> heap creation” (JDK-8130459)
>>>
>>>
>>> Changes in webrev3 compared to webrev2:
>>>
>>> - Adding DBL_MAX - decided to let Sangheon handle this as part of his
>>> effort to add missing GC flags ranges/constraints
>>>
>>> - Do not bother checking for NULL in
>>> CommandLineFlagRangesList::check_ranges() and
>>> CommandLineFlagConstraintsList::check_constraints() - we crash on
>>> 32bit platforms if we do this, so can't do it. The problem exists
>>> because there are certain flags (ie. lp64_product) that have ranges
>>> (ie. ObjectAlignmentInBytes) that are defined as constants on 32 bit
>>> platforms, but still register as flags with ranges. I added comments
>>> to the code in question.
>>>
>>> - Implement missing range/constraints check in "int" and "uint"
>>> CommandLineFlagsEx::*AtPut() APIs
>>>
>>> - Factored out common error printing code
>>>
>>> - Use "ShouldNotReachHere()" in Flag::flag_error_str()
>>>
>>> - Consistent constraint error message formating
>>>
>>> - Consistent placement of "verbose" argument in APIs
>>>
>>> - Fix other APIs that used to pass value by reference
>>>
>>> - Merged with Roland Westlin fix for "CICompilerCount=1 when tiered is
>>> off is not allowed any more" (JDK-8130858)
>>>
>>>
>>> This webrev passes "JPRT -testset hotspot” and “RBT vm.quick testlist”
>>>
>>>
>>> References:
>>>
>>>    JEP 245: https://bugs.openjdk.java.net/browse/JDK-8122937
>>>        bug: https://bugs.openjdk.java.net/browse/JDK-8112746
>>>     webrev: http://cr.openjdk.java.net/~gziemski/8112746_rev3/
>>>
>>>
>>> cheers
>>>
>>
>>
>>