RFR (M): 8142510: -XX:+PrintFlagsRanges should print default range value for those flags that have constraint and an implicit range.

[Gerard Ziemski]

hi Dmitry,

Thank you very much for the review. Please see my answers inline:

> On Mar 11, 2016, at 2:46 PM, Dmitry Dmitriev <dmitry.dmitriev at oracle.com> wrote:
> 
> Hi Gerard,
> 
> Few comments, otherwise looks good. Thank you for doing that!
> 1) src/share/vm/runtime/globals.cpp
> I think that it's unnecessary to declare string_length and range_string as static in create_range_str function.

The intention is to reuse the buffer and only allocate more memory if absolutely necessary. The “static” designation allows us to cache the buffer between calls.


> 
> 2) src/share/vm/runtime/commandLineFlagConstraintList.cpp
> New function CommandLineFlagConstraintList::find looks very similar to the existing one CommandLineFlagConstraintList::find_if_needs_check. I think that you can remove duplicated logic by refactoring CommandLineFlagConstraintList::find_if_needs_check as follows:
>  CommandLineFlagConstraint* CommandLineFlagConstraintList::find_if_needs_check(const char* name) {
>    CommandLineFlagConstraint* found = find(name);
>      if ((found != NULL) &&
>          (found->type() > _validating_type)) {
> ***        Please add appropriate comment here, why found is set back to NULL  ***
>        found = NULL;
>      }
>    return found;
>  }
> 

A good suggestion.

I will update and post 2nd webrev shortly.


cheers


> Thanks,
> Dmitry
> 
> On 09.03.2016 20:33, Gerard Ziemski wrote:
>> hi all,
>> 
>> Please review this enhancement to Command Line Options Validation JEP-245, which prints default ranges for those flags, that only have constraints (ie. no range, but a constraint, implies default range)
>> 
>> bug https://bugs.openjdk.java.net/browse/JDK-8142510
>> webrev http://cr.openjdk.java.net/~gziemski/8142510_rev0
>> 
>> tested with JPRT hotspot, RBT hotspot/test/runtime and local test/runtime/CommandLine/OptionsValidation
>> 
>> 
>> cheers
>