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

gerard ziemski gerard.ziemski at oracle.com
Tue Aug 4 19:30:38 UTC 2015


Big thank you for this review!Comments in-line.


On 08/04/2015 01:46 PM, Kim Barrett wrote:
> On Jul 31, 2015, at 2:04 PM, gerard ziemski <gerard.ziemski at oracle.com> wrote:
>> Please review this webrev 2 of the follow-up fixes including:
>>
>> […]
>>         bug:
>> https://bugs.openjdk.java.net/browse/JDK-8112746
>>
>>      webrev:
>> http://cr.openjdk.java.net/~gziemski/8112746_rev2/
> A couple more comments:
>
> ------------------------------------------------------------------------------
>> - 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)
> In src/share/vm/runtime/globals.cpp
>
> I think all the (static) apply_constraint_and_check_range_xxx helper
> functions should be similarly changed to take the new_value argument
> by value.

Fixed.


>
> ------------------------------------------------------------------------------
> src/share/vm/runtime/globals.cpp
>
>   854 Flag::Error CommandLineFlagsEx::intAtPut(CommandLineFlagWithType flag, int value, Flag::Flags origin) {
>
> This function is missing call to apply_constraint_and_check_range_int.
>
> Similarly,
>
>   900 Flag::Error CommandLineFlagsEx::uintAtPut(CommandLineFlagWithType flag, uint value, Flag::Flags origin) {
>
> is missing call to apply_constraint_and_check_range_uint.

Big thanks for this one - I was just investigating why I didn't see my 
intx constraint functionsfiring. Fixed.


cheers


More information about the hotspot-dev mailing list