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

[gerard ziemski]

Thank you Coleen!


On 08/07/2015 06:21 PM, Coleen Phillimore wrote:
>
> 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
>>>>
>>>
>>>
>>>
>
>
>