RFR(XS): 7154809 JDI: update JDI/JDB debugee commandline option parsing (allow nested comma delimited options) + sponsor request
Markus Grönlund
markus.gronlund at oracle.com
Wed Mar 28 14:13:03 PDT 2012
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/
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 serviceability-dev
mailing list