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

Thu Apr 5 04:43:16 PDT 2012

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/

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