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
-----Original Message-----
From: Daniel D. Daugherty
Sent: den 27 mars 2012 16:42
To: [email protected]; hotspot-runtime-
[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 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: [email protected]; hotspot-runtime-
[email protected]
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: [email protected]; hotspot-runtime-
[email protected]
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: [email protected]; hotspot-runtime-
[email protected]
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 ...
