On 19/01/2012 10:18 PM, Frederic Parain wrote:
On 01/19/12 01:08 PM, David Holmes wrote:
Okay that all makes sense now.

But does that mean other parse routines have a similar issue eg:

39 template <> void DCmdArgument<jlong>::parse_value(const char* str,
40 size_t len, TRAPS) {
41 if (sscanf(str, INT64_FORMAT, &_value) != 1) {
42 THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
43 "Integer parsing error in diagnostic command arguments");
44 }
45 }

sscanf will stop parsing as soon as it finds a character
which cannot be used to create an integer.

Ah - right!

Unless someone as the bad idea to use a digit character
as delimiter, it should be safe.

Yeah that would be a bad idea :)

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.

David
-----

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







Reply via email to