src/share/vm/services/diagnosticArgument.cpp
Maybe the following comment before line 65:
// len is the length of the current token starting at str
However, that's just a suggestion. Thumbs up.
Dan
On 1/20/12 7:45 AM, Frederic Parain wrote:
Thanks David,
I just need a second reviewer and I'll push the fix.
Fred
On 1/20/12 12:18 PM, David Holmes wrote:
On 19/01/2012 10:33 PM, Frederic Parain wrote:
On 01/19/12 01:28 PM, David Holmes wrote:
However, I'm not sure that trailing characters are properly
detected. Do you consider that as a show stopper?
Depends on exactly what you mean by that. If extra junk characters are
ignored rather than causing an error that's okay to me.
They will be silently ignored.
Not a showstopper in that case.
So this looks good to go.
David
-----
Fred
Fred
Fred
On 01/19/2012 11:48 AM, Dmitry Samersoff wrote:
Frederic,
Sorry, of course
strncasecmp(str, "true", 4) == 0&& str[4] == 0
-Dmitry
On 2012-01-19 14:26, Frederic Parain wrote:
strncasecmp(str, "true", 4) == 0 would accept
arguments like this:
-all=truefalse
which are not valid.
Fred
On 01/19/12 11:22 AM, Dmitry Samersoff wrote:
Frederic,
I think explicit check for len is not necessary,
strncasecmp(str, "true", 4) == 0
is enough.
-Dmitry
On 2012-01-19 13:59, Frederic Parain wrote:
This is a small fix (2 lines) to fix an issue with the
parsing of boolean arguments by diagnostic commands
CR is not available on bugs.sun.com yet, but the description
says that the string comparisons to identify "true" or "false"
values doesn't take into account the length of the argument
being parse.
The suggested fix is:
--- old/src/share/vm/services/diagnosticArgument.cpp Thu
Jan 19
10:36:10 2012
+++ new/src/share/vm/services/diagnosticArgument.cpp Thu
Jan 19
10:36:10 2012
@@ -62,9 +62,9 @@
if (len == 0) {
set_value(true);
} else {
- if (strcasecmp(str, "true") == 0) {
+ if (len == strlen("true")&& strncasecmp(str, "true", len) ==
0) {
set_value(true);
- } else if (strcasecmp(str, "false") == 0) {
+ } else if (len == strlen("false")&& strncasecmp(str,
"false",
len)
== 0) {
set_value(false);
} else {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
Webrev:
http://cr.openjdk.java.net/~fparain/7131346/webrev.00/
Thanks,
Fred
