On 01/19/12 12:39 PM, Robert Ottenhag wrote:
However, I do not get it, why cannot ordinary strcasecmp(str, "true") be
used here, is it problem with an input string that is not null
terminated, or is it some other problem?

Correct. The parsing must be applied only to a section of str which
is not null terminated.

Reading the bug, the failing input is "-all=true", but how is that a
problem, assuming the upper layer code has separated "-all=" from "true"
before parsing the value?


The CR is missing some information (I've received a more detailed
description of the failure by e-mail).

The failing input is in fact "-all=true filename.hprof"

Where the value of the boolean argument is followed by
a delimiter (a space in this case) and another argument.

When the parsing method is invoked, str points to the 't' of
"true" and len is 4:

Using strcasecmp:
   strcasecmp("true filename.hprof","true") returns a positive number

Using strncasecmp:
   strncasecmp("true filename.hprof","true",len) returns zero

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






--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: [email protected]

Reply via email to