[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: RFR(XS): 7154809 JDI: update JDI/JDB debugee commandline option parsing (allow nested comma delimite
From: daniel.daugherty () oracle ! com (Daniel D ! Daugherty)
Date: 2012-03-29 20:30:21
Message-ID: 4F74C65C.7000905 () oracle ! com
[Download RAW message or body]
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 ...
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic