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: [email protected] > Cc: [email protected] > 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; [email protected]; hotspot- > >> [email protected] > >> 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
