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

[sangheon.kim]

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.

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
>