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
Thu Apr 5 23:25:54 PDT 2012
Thanks Dan,
I intend to put down Dan, Dmitry and Staffan as reviewers for this change.
Thank you
Markus
> -----Original Message-----
> From: Daniel D. Daugherty
> Sent: den 6 april 2012 03:49
> 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
>
> On 4/5/12 5:43 AM, Markus Grönlund wrote:
> > Dan, Dmitry, all,
> >
> > Thanks for the feedback so far, I have updated the change, please see
> this update:
> >
> > Webrev:
> > http://cr.openjdk.java.net/~mgronlun/7154809/webrev06/
>
> Thumbs up!
>
> src/share/classes/com/sun/tools/example/debug/tty/VMConnection.java
>
> nit line 119: should be 'chars' (not a contraction)
>
> nit line 184: indented 1 space too much
>
> nit line 226: space around '+'
>
> Dan
>
>
> > Bug:
> > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7154809
> >
> >
> >
> > The use case we are trying to support is the parsing of VM Options
> being passed in via the "options="
> >
> > These VM options will either be:
> >
> > 1. undecorated (-XX:+PrintVMOptions)
> > 2. double quoted ("-XX:+PrintVMOptions")
> > 3. single quoted ('-XX:+PrintVMOptions')
> > 4. nested quoted ('"-XX:+PrintVMOptions"', or "'-
> XX:+PrintVMOptions'")
> >
> > Above change results in:
> >
> > -XX:+PrintVMOptions
> >
> > For all the above.
> >
> >
> > Example (the extra \ qualifiers is to avoid shell resolution for
> windows commandline):
> >
> > java TestApplication \"-client\" \"-Xmixed\" \"-XX:+PrintVMOptions\"
> \"'-XX:ExtendedExampleOptions=value1=true,value2=true,value3=false'\"
> > Printing raw arguments:
> > "-client"
> > "-Xmixed"
> > "-XX:+PrintVMOptions"
> > "'-XX:ExtendedExampleOptions=value1=true,value2=true,value3=false'"
> >
> > Printing raw arguments stringed together: "-client" "-Xmixed" "-
> XX:+PrintVMOptions" "'-
> XX:ExtendedExampleOptions=value1=true,value2=true,value3=false'"
> >
> > strings after splitStringAtNonEnclosedWhitespace():
> >
> > "-client"
> > now sending string: "-client" off to isEnclosed()
> > After processing string in isEnclosed:
> > -client
> >
> > "-Xmixed"
> > now sending string: "-Xmixed" off to isEnclosed()
> > After processing string in isEnclosed():
> > -Xmixed
> >
> > "-XX:+PrintVMOptions"
> > now sending string: "-XX:+PrintVMOptions" off to isEnclosed()
> > After processing string in isEnclosed():
> > -XX:+PrintVMOptions
> >
> > "'-XX:ExtendedExampleOptions=value1=true,value2=true,value3=false'"
> > now sending string: "'-
> XX:ExtendedExampleOptions=value1=true,value2=true,value3=false'" off to
> isEnclosed()
> > After processing string in isEnclosed():
> > -XX:ExtendedExampleOptions=value1=true,value2=true,value3=false
> >
> > Final result
> > -client -Xmixed -XX:+PrintVMOptions -
> XX:ExtendedExampleOptions=value1=true,value2=true,value3=false
> >
> >
> > Thank you
> > Markus
> >
> >
> > p.s.
> >
> > Dan, thanks for the esoteric example. The output from the example
> string (using debug version of code):
> >
> > The following is a single quote: ' Do you see it?
> >
> > Is (depending on how you pass it in (quoted or unquoted)):
> >
> > D:\devjava\1.6.0\projects\enclosed\source>java TestApplication a
> single quote: ' do you see it?
> >
> > Printing raw arguments:
> >
> > a
> > single
> > quote:
> > '
> > do
> > you
> > see
> > it?
> >
> > Printing raw arguments stringed together: a single quote: ' do you
> see it?
> >
> > strings after splitStringAtNonEnclosedWhitespace:
> >
> > a
> > now sending string: a off to isEnclosed()
> > After processing string in isEnclosed():
> > a
> >
> > single
> > now sending string: single off to isEnclosed()
> > After processing string in isEnclosed():
> > single
> >
> > quote:
> > now sending string: quote: off to isEnclosed()
> > After processing string in isEnclosed():
> > quote:
> >
> > '
> > now sending string: ' off to isEnclosed()
> > After processing string in isEnclosed():
> > '
> >
> > do
> > now sending string: do off to isEnclosed()
> > After processing string in isEnclosed:
> > do
> >
> > you
> > now sending string: you off to isEnclosed()
> > After processing string in isEnclosed():
> > you
> >
> > see
> > now sending string: see off to isEnclosed()
> > After processing string in isEnclosed():
> > see
> >
> > it?
> > now sending string: it? off to isEnclosed()
> > After processing string in isEnclosed():
> > it?
> >
> > Final result
> > a single quote: ' do you see it?
> >
> > D:\devjava\1.6.0\projects\enclosed\source>java TestApplication \"a
> single quote: ' do you see it?\"
> >
> > Printing raw arguments:
> > "a
> > single
> > quote:
> > '
> > do
> > you
> > see
> > it?"
> >
> > Printing raw arguments stringed together: "a single quote: ' do you
> see it?"
> >
> > strings after splitStringAtNonEnclosedWhitespace
> > "a single quote: ' do you see it?"
> > now sending string: "a single quote: ' do you see it?" off to
> isEnclosed()
> > After processing string in isEnclosed():
> > a single quote: ' do you see it?
> >
> > Final result
> > a single quote: ' do you see it?
> >
> >
> > D:\devjava\1.6.0\projects\enclosed\source>java TestApplication 'a
> single quote: \" do you see it?'
> >
> > Printing raw arguments
> > 'a
> > single
> > quote:
> > "
> > do
> > you
> > see
> > it?'
> > Printing raw arguments stringed together: 'a single quote: " do you
> see it?'
> > strings after splitStringAtNonEnclosedWhitespace
> > 'a single quote: " do you see it?'
> > now sending string: 'a single quote: " do you see it?' off to
> isEnclosed()
> > After processing string in isEnclosed:
> > a single quote: " do you see it?
> >
> > Final result
> > a single quote: " do you see it?
> >
> > Cheers
> > Markus
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >> -----Original Message-----
> >> From: Daniel D. Daugherty
> >> Sent: den 29 mars 2012 22:30
> >> To: serviceability-dev at openjdk.java.net
> >> Cc: 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/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
More information about the serviceability-dev
mailing list