RFR(XS): 7154809 JDI: update JDI/JDB debugee commandline option parsing (allow nested comma delimited options) + sponsor request
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Mar 27 12:29:25 PDT 2012
On 3/27/12 11:56 AM, 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/
src/share/classes/com/sun/tools/example/debug/tty/VMConnection.java
I like the newer version and the concept of "enclosing quotes".
One thing I'm not quite sure about is the use of 'split()'. What
will the new logic do with the following value?
"<word1> <word2>"
I'm pretty sure the split() call will break the above into two
tokens: '"<word1>' and '<word2>"'. The subsequent calls to
isEnclosed() will return false and we won't strip the double
quotes. Maybe I'm misunderstanding how 'split("\\s+")' works...
BTW, this options parsing stuff is exceedingly difficult to get
working right and working the same on all platforms.
Dan
> 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 ...
More information about the serviceability-dev
mailing list