RFR(XS): 7154809 JDI: update JDI/JDB debugee commandline option parsing (allow nested comma delimited options) + sponsor request

[Dmitry Samersoff]

Markus,

Looks good for me!

-Dmitry

On 2012-03-27 21:56, Markus Grönlund wrote:
> Hi again all,
> 
> Thanks for the input Dan and Dmitry.
> 
> I took a closer look at this, we can actually pass options to the VM which could involve filenames and these *could* have quotation qualifiers embedded (ccstr type parameters); this means the original suggestion will not work correctly, in doing value.replaceAll("['\"]", ""), as it will also remove all quotations for any embedded VMOptions of ccstr type...
> 
> Neither will Dmitrys suggestion of removing just the edge quotations work, at least not as-is, it would need foreach value iteration.
> 
> I have updated the webrev with such a modification (which will recursively remove quotations from outside-in for each value). This will allow for "'test'", '" test "', "" test "", ''test'', "test", 'test' ...etc.
> 
> Please see updated webrev here:
> 
> http://cr.openjdk.java.net/~mgronlun/7154809/webrev04/
> 
> Thanks you so much for your help
> 
> Cheers
> Markus
> 
> 
> 
>   
> 
>> -----Original Message-----
>> From: Daniel D. Daugherty
>> Sent: den 27 mars 2012 16:42
>> To: serviceability-dev at openjdk.java.net; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: Re: RFR(XS): 7154809 JDI: update JDI/JDB debugee commandline
>> option parsing (allow nested comma delimited options) + sponsor request
>>
>> On 3/27/12 5:51 AM, Markus Grönlund wrote:
>>> Hi again,
>>>
>>> Updated webrev02 for conditional replacement only for "options".
>> Thanks Dmitry!
>>>
>>> http://cr.openjdk.java.net/~mgronlun/7154809/webrev02/
>>
>> src/share/classes/com/sun/tools/example/debug/tty/VMConnection.java
>>      line 116 - why a JavaDoc style comment beginning?
>>
>>      line 121 - This removes all instances of single and double quotes
>>          which might be OK given that this is for options only. For
>>          example:
>>
>>              var="don't tread on me"
>>
>>          will get morphed into:
>>
>>              var=dont tread on me
>>
>>          However, I don't know of any options where we need to preserve
>>          single instances of either single or double quotes.
>>
>> I think this is OK as is. Thumbs up.
>>
>> Dan
>>
>>
>>>
>>> Thanks
>>> Markus
>>>
>>>> -----Original Message-----
>>>> From: Markus Grönlund
>>>> Sent: den 27 mars 2012 12:56
>>>> To: Dmitry Samersoff
>>>> Cc: serviceability-dev at openjdk.java.net; hotspot-runtime-
>>>> dev at openjdk.java.net
>>>> Subject: RE: RFR(XS): 7154809 JDI: update JDI/JDB debugee
>> commandline
>>>> option parsing (allow nested comma delimited options) + sponsor
>> request
>>>>
>>>> Hi Dmitry,
>>>>
>>>> That's a really good reflection! Thank you for this.
>>>>
>>>> I will make the replacement logic conditional and only apply to the
>>>> "options" token instead, this will leave any eventual "' in file
>> names
>>>> alone.
>>>>
>>>> Thanks!
>>>>
>>>> Markus
>>>>
>>>>> -----Original Message-----
>>>>> From: Dmitry Samersoff
>>>>> Sent: den 27 mars 2012 12:22
>>>>> To: Markus Grönlund
>>>>> Cc: serviceability-dev at openjdk.java.net; hotspot-runtime-
>>>>> dev at openjdk.java.net
>>>>> Subject: Re: RFR(XS): 7154809 JDI: update JDI/JDB debugee
>> commandline
>>>>> option parsing (allow nested comma delimited options) + sponsor
>>>> request
>>>>> Markus,
>>>>>
>>>>>> main=some.namespace.java.class,
>>>>> " (or ') is valid character in a file name so it's better not to
>>>> change
>>>>> it.
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>>
>>>>> On 2012-03-27 14:08, Markus Grönlund wrote:
>>>>>> Dmitry,
>>>>>>
>>>>>> Thanks, yes I made it very optimistic at this point.
>>>>>>
>>>>>> Maybe could be made more intelligent. However, I didn’t see the
>>>> need
>>>>> for it really as no existing option values are allowed to contain '
>> '
>>>>> in them.
>>>>>> The java debugger has a certain amount of predefined delimited set
>>>> of
>>>>> options, for example:
>>>>>> vmexec=java,
>>>>>>
>>>>>> options= "-client" "-XX:+PrintVMOptions",
>>>>>>
>>>>>> main=some.namespace.java.class,
>>>>>>
>>>>>>
>>>>>> /Markus
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Dmitry Samersoff
>>>>>>> Sent: den 27 mars 2012 11:59
>>>>>>> To: Markus Grönlund
>>>>>>> Cc: serviceability-dev at openjdk.java.net; hotspot-runtime-
>>>>>>> dev at openjdk.java.net
>>>>>>> Subject: Re: RFR(XS): 7154809 JDI: update JDI/JDB debugee
>>>>> commandline
>>>>>>> option parsing (allow nested comma delimited options) + sponsor
>>>>> request
>>>>>>> Markus,
>>>>>>>
>>>>>>> Your changes strip comma in the middle of argument as well:
>>>>>>>
>>>>>>> i.e.
>>>>>>>
>>>>>>> String value="\'Bl\"a\'";
>>>>>>> System.out.println( value.replaceAll("['\"]", "") );
>>>>>>>
>>>>>>> Prints:  Bla
>>>>>>>
>>>>>>> Is it intentional?
>>>>>>>
>>>>>>> -Dmitry
>>>>>>>
>>>>>>>
>>>>>>> On 2012-03-27 12:49, Markus Grönlund wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> I would like to ask for a review:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Webrev: http://cr.openjdk.java.net/~mgronlun/7154809/webrev01/
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Bug/CR: 7154809 JDI: update JDI/JDB debugee commandline option
>>>>>>> parsing
>>>>>>>> (allow nested comma delimited options)
>>>>>>>>
>>>>>>>> (bug is not yet published on bugs.sun.com, I am attaching a copy
>>>> of
>>>>>>> the
>>>>>>>> bug description to the mail below)
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Synopsis: 7154809 JDI: update JDI/JDB debugee commandline option
>>>>>>> parsing
>>>>>>>> (allow nested comma delimited options)
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Description:
>>>>>>>>
>>>>>>>> Passing in a double quoted value, such as "-XX:+PrintVMOptions"
>>>> to
>>>>>>> the
>>>>>>>> debugee works today. But only because double-quoted options can
>>>> be
>>>>>>>> passed directly onto the actual VM command-line (where it is
>>>>> stripped
>>>>>>> by
>>>>>>>> the VM). What does not work is passing the debugee single-quoted
>>>>>>> values
>>>>>>>> such as '-XX:+PrintVMOptions', although the regexp in
>>>> VMConnection
>>>>>>> works
>>>>>>>> ok for proper comma-delimting of option separation. However,
>>>> single
>>>>>>>> quoted values cannot be passed on directly to the VM, since the
>>>> VM
>>>>>>> does
>>>>>>>> not strip these single quotes. Also, values which are contained
>>>>>>> inside
>>>>>>>> nested quotes like “” value “” and “’ value ‘”  will not work
>> for
>>>>> the
>>>>>>>> same reason.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> To allow for more flexibility in passing delimited values (which
>>>>>>> needs
>>>>>>>> to be quoted), VMConnection should strip out any quote
>> qualifiers
>>>>>>>> (single and/or double quotes) before passing the options onto
>> the
>>>>> VM.
>>>>>>>> Besides adding more flexibility in option passing, this also
>>>> allows
>>>>>>> for
>>>>>>>> more reliable command-line argument handling/processing, as
>>>> options
>>>>>>> are
>>>>>>>> always passed non-quoted to the VM.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Small fix to VMConnection.java is considered safe and backwards
>>>>>>> compatible.
>>>>>>>>
>>>>>>>>
>>>>>>>> I would also kindly ask for a sponsor to help me with this
>>>> putback.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thank you
>>>>>>>>
>>>>>>>> Markus
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Dmitry Samersoff
>>>>>>> Java Hotspot development team, SPB04
>>>>>>> * There will come soft rains ...
>>>>>
>>>>> --
>>>>> Dmitry Samersoff
>>>>> Java Hotspot development team, SPB04
>>>>> * There will come soft rains ...


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...