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
Thu Mar 29 13:30:21 PDT 2012
On 3/28/12 3:13 PM, Markus Grönlund wrote:
> Dan, Dmitry, everybody,
>
> Thank you so far for your input.
>
> Dan, you are absolutely right in your reflection about "<word1> <word2>" combined with 'split("\\s+")' regexp - this will split the strings to"<word1> and<word2>" which does not work. We should also try to cover this.
>
> This turned out a lot trickier than I originally thought - I have tried to come up with some good regexps that would combine the combination of what's needed (nested spaces, nested quotations (different quotations), several levels...), however I did not get it to work correctly in all combinations with the regexps - I eventually decided to take control over this down to the char level - hence this suggestion with a C-like array iteration (maybe slow yeah, however I found this so much more debuggable and actually a chance to see what's going on. Also, this parsing for the "values"-string associated with the "option=" is only parsed once at startup, just when getting the correct VM Option parameters for/of the debugee).
>
> Updated webrev:
> http://cr.openjdk.java.net/~mgronlun/7154809/webrev05/
src/share/classes/com/sun/tools/example/debug/tty/VMConnection.java
nit lines 118-119: extra space after '*'
line 178: what happens when null is passed to format()?
lines 182, many others - missing space after '//'
line 217 - please consider changing comment to:
// this DOUBLEQ does not complete this enclosing - skip to next
line 224: please change comment to match line 256:
//set up the target char for ending enclosing
line 229 - please consider changing comment to:
// this DOUBLEQ does not start a new enclosing - skip to next
line 233: add "is there" to beginning of comment to match line 201
line 249 - please consider changing comment to:
// this SINGLEQ does not complete this enclosing - skip to next
line 261 - please consider changing comment to:
// this SINGLEQ does not start a new enclosing - skip to next
line 286: This check might be hit for an input string that looks like:
The following is a single quote: ' Do you see it?
Pretty bogus example so throwing that exception might be the
right thing to do. The idea I have is that if you have a
singleton DOUBLEQ or SINGLEQ in the input, then you don't
have an enclosing state... May not be worth the hassle...
Dan
> There was eventually some more code ending up in this one (trying to be very explicit here) - it might make more sense to refactor this to outside of VMConnection.java someplace - if you have any ideas...
>
> Some of the word combinations supported with this change:
>
> "'<word1> <word2>'"<word3> '"<word4> <word5>"'"<word6> <word7>" '<word8> <word9>'<word10> "<wor"d11>" '<word'12>''<wor"d11>' "<wor"d'12>" "<word13> " ....
>
> I think I should need to track the changes to the options passing/parsing test code in nsk testbase as well in a separate bug for correlation with these changes - might make more sense. I will file a test bug on the updates needed on the test side of things.
>
> Thanks again for your help
> Markus
>
>
>
>> -----Original Message-----
>> From: Daniel D. Daugherty
>> Sent: den 27 mars 2012 21:29
>> To: Markus Grönlund
>> Cc: Dmitry Samersoff; 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 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 hotspot-runtime-dev
mailing list