[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(one-liner): 8191707: Options with invalid values are incorrectly treated as obsolete and ign
From:       David Holmes <david.holmes () oracle ! com>
Date:       2017-11-28 7:35:55
Message-ID: 90437262-53cd-4d49-2a9b-ed893999e261 () oracle ! com
[Download RAW message or body]

On 23/11/2017 10:19 AM, Daniel D. Daugherty wrote:
> On 11/22/17 3:24 PM, Robbin Ehn wrote:
>> Hi all, please review:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8191707
>>
>> Test: test/hotspot/jtreg/runtime/CommandLine/ and tier 1-5 with no 
>> unexpected failure.
>>
>> Contributed by David Holmes.
>>
>> Looks good, thanks for fixing!
> 
> Thumbs up on the fix.
> 
> What I don't understand is why this logic did not fall over before now.

I think we previously always hit the "undefined" case for the obsolete 
version, when dealing with a flag that could have an invalid value. All 
of the other recently added deprecated flags were booleans.

Of course that means we are missing some test coverage here.

David

> Dan
> 
>>
>> /Robbin
>>
>>
>> diff -r 2cd1c2b03782 src/hotspot/share/runtime/arguments.cpp
>> --- a/src/hotspot/share/runtime/arguments.cpp       Wed Nov 22 01:12:23 
>> 2017 -0800
>> +++ b/src/hotspot/share/runtime/arguments.cpp       Wed Nov 22 21:20:42 
>> 2017 +0100
>> @@ -493,15 +493,15 @@
>>   }
>>
>>   bool Arguments::is_obsolete_flag(const char *flag_name, JDK_Version* 
>> version) {
>>      assert(version != NULL, "Must provide a version buffer");
>>      SpecialFlag flag;
>>      if (lookup_special_flag(flag_name, flag)) {
>>          if (!flag.obsolete_in.is_undefined()) {
>> -           if (version_less_than(JDK_Version::current(), flag.expired_in)) {
>> +           if (!version_less_than(JDK_Version::current(), 
>> flag.obsolete_in)) {
>>                  *version = flag.obsolete_in;
>>                  return true;
>>              }
>>          }
>>      }
>>      return false;
>>   }
> 
[prev in list] [next in list] [prev in thread] [next in thread]