Corrected: RFR 8059557 (XL): Validate JVM Command-Line Flag Arguments

[Gerard Ziemski]

hi Coleen,

Thank you for taking the time to review this considerable change. I have 
responded to your points below:

On 5/15/2015 3:49 PM, Coleen Phillimore wrote:
>
> Gerard,
> This is significant work!  The macro re-expansions are daunting but 
> the simpler user interface makes it worth while.  Someone better at 
> macros should review this in more detail to see if there's any 
> gotchas, especially wrt to C++11 and beyond. Hopefully there aren't 
> any surprises.  I think there's some things people should know about 
> globals.hpp:
>
> *+   /*  NB: The default value of UseLinuxPosixThreadCPUClocks may be   */ \*
> *+   /* overridden in Arguments::parse_each_vm_init_arg.                */ \*
> *     product(bool, UseLinuxPosixThreadCPUClocks, true,                     \*
> *             "enable fast Linux Posix clocks where available")             \*
> *   /*  NB: The default value of UseLinuxPosixThreadCPUClocks may be        \*
> *       overridden in Arguments::parse_each_vm_init_arg.  */                \*
> It looks like if you have a comment for an option, do you need to have 
> it above the option, or is this just nicer?

To make it nicer, but more importantly to make it more consistent.

>
> In 
> http://cr.openjdk.java.net/~gziemski/8059557_rev0/src/share/vm/runtime/globals.cpp.udiff.html
>
> This should be out->print_cr()
>
> *+   if (printRanges == false) {*
> *       out->print_cr("[Global flags]");*
> *+   } else {*
> *+     tty->print_cr("[Global flags ranges]");*
> *+   }*
> *+*

Done.

> There's some ifdef debugging code left in but I think that's ok for 
> now, because it's not much and may be helpful but not helpful enough 
> in the long run to add a XX:OnlyPrintProductFlags option.
>
> The name checkAllRangesAndConstraints should be 
> check_all_ranges_and_constraints as per the coding standard, but the 
> constraint functions in
> http://cr.openjdk.java.net/~gziemski/8059557_rev0/src/share/vm/runtime/commandLineFlagConstraintsGC.hpp.html
> seem okay in mixed case so you can find them when you're grepping for 
> the flag.

Done.


cheers