Disallow flags with extra characters appended

[David Holmes]

On 11/12/2014 10:26 AM, Jesper Wilhelmsson wrote:
> Update:
>
> The bug has now been reopened with Christian Törnqvist's blessing.
>
> David, did you have further comments on the updated version?

No. I just replied to the review thread. Sorry it slipped through the 
cracks.

David

> (Link from the review thread in the runtime list:
> http://cr.openjdk.java.net/~jwilhelm/6522873/webrev.00/ )
>
> Thanks!
> /Jesper
>
>
> Jesper Wilhelmsson skrev 5/12/14 14:39:
>> Hi,
>>
>> In the hope that JDK-6522873 will be reopened I'm sending a proper RFR
>> to the
>> runtime list. Thanks David for your initial review, I have updated the
>> change as
>> you suggested. I also found a few more flags that should be covered by
>> this
>> change. A full list of flags is available in the RFR.
>>
>> Thanks,
>> /Jesper
>>
>> Ioi Lam skrev 5/12/14 10:43:
>>>
>>> On 12/5/14, 1:38 AM, David Holmes wrote:
>>>> On 5/12/2014 6:27 PM, Jesper Wilhelmsson wrote:
>>>>> Hi David,
>>>>>
>>>>> Thanks for the pointer to JDK-6522873!
>>>>>
>>>>> Not fixing this with the motivation "due to the risk of breaking
>>>>> existing applications/scripts that already has the incorrect options
>>>>> specified" is ridiculous imho.
>>>>>
>>>>> As I wrote in the bug just now:
>>>>> We have to stop being so scared about fixing bugs. Fixing a bug like
>>>>> this in a major version (9) should cause minimal problems since people
>>>>> will have to re-certify their applications and change the command
>>>>> lines
>>>>> anyway to upgrade from 8 to 9.
>>>>>
>>>>> Fixing this bug will make some people go "Oh, I made a typo" and
>>>>> fix it.
>>>>> Not fixing this bug will make people go "Hey, we have been running
>>>>> with
>>>>> the wrong settings for two years!! Why didn't this $#%@#$# VM say
>>>>> something?!?!"
>>>>
>>>> I tend to agree that fixing this in a major release like 9 seems to
>>>> be not
>>>> unreasonable. However it is easy to under estimate the compatibility
>>>> aspect of
>>>> simple changes - even though that can be very frustrating. Given
>>>> most of the
>>>> -X options are pretty useless I don't see this as likely being a major
>>>> problem. Even so I come down slightly on the side of fixing it.
>>>>
>>>> And as you note 4514888 already fixed this in 5 and we seem to have
>>>> regressed
>>>> since then.
>>>>
>>> I think it's reasonable to fix for 9. Since there's a compatibility
>>> concern, we
>>> should get a CCC approval. Also, maybe a
>>> -XX:+DontComplainAboutRubbishAfterOptionsSinceIamTooLazyToFixMyScripts flag
>>> for
>>> backwards compatibility? People who are truly lazy can just add this
>>> in their
>>> JAVA_OPTS environment variable.
>>>
>>> - Ioi
>>>> David
>>>>
>>>>> Dear runtime team, please consider reopening this bug.
>>>>>
>>>>> Thanks,
>>>>> /Jesper
>>>>>
>>>>>
>>>>> David Holmes skrev 5/12/14 04:46:
>>>>>> Hi Jesper,
>>>>>>
>>>>>> On 5/12/2014 11:38 AM, Jesper Wilhelmsson wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Today some (not all) flags are accepted even though they have random
>>>>>>> characters appended to them. Some examples are -Xconcgc, -Xcomp,
>>>>>>> -Xboundthreads, -XX:+AlwaysTenure etc which will also be accepted
>>>>>>> when
>>>>>>> written for instance -Xconcgcnoway, -Xcomposer,
>>>>>>> -Xboundthreadstodogs or
>>>>>>> -XX:+AlwaysTenureAtBlueMoon
>>>>>>>
>>>>>>> There is a potential problem here since we will also accept
>>>>>>> things like
>>>>>>> -XX:+ExtendedDTraceProbes-XX:+UseG1GC without saying a word (and of
>>>>>>> course without running with G1).
>>>>>>>
>>>>>>> I have a suggestion for a fix here:
>>>>>>> http://cr.openjdk.java.net/~jwilhelm/commandLineFlag/webrev.00/
>>>>>>>
>>>>>>> Would this be an acceptable solution?
>>>>>>
>>>>>> I'm somewhat surprised the single name version of match_option didn't
>>>>>> also have the _tail_allowed flag - seems rather unbalanced. But what
>>>>>> you have is cleaner I think. Though I would suggest moving you new
>>>>>> version:
>>>>>>
>>>>>>  static bool match_option(const JavaVMOption *option, const char*
>>>>>> name) {
>>>>>>
>>>>>> to immediately after the tail version (and before the _tail_allowed
>>>>>> multi-name version), which a suitable comment added.
>>>>>>
>>>>>> That said ...
>>>>>>
>>>>>>> I couldn't find one, but since this has been around for quite
>>>>>>> some time
>>>>>>> I wonder if there is a bug for this already. If not I'll create one.
>>>>>>
>>>>>> ... this has already been rejected
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-6522873
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> Thanks,
>>>>>>> /Jesper
>>>>>>>
>>>>>
>>>