[webkit-reviews] review requested: [Bug 10820] Add StringImpl::toDouble() and remove uses of .deprecatedString().toDouble() : [Attachment 10778] Non-broken patch

[bugzilla-request-daemon at opendarwin.org]

Anders Carlsson <acarlsson at apple.com> has asked  for review:
Bug 10820: Add StringImpl::toDouble() and remove uses of
.deprecatedString().toDouble()
http://bugzilla.opendarwin.org/show_bug.cgi?id=10820

Attachment 10778: Non-broken patch
http://bugzilla.opendarwin.org/attachment.cgi?id=10778&action=edit

------- Additional Comments from Anders Carlsson <acarlsson at apple.com>
I found two problems with the toDouble implementation:

+double StringImpl::toDouble(bool* ok) const
+{
+    if (empty()) {

this should be isEmpty(). empty() returns the special empty string singleton so
this is always true.

+	 if (ok)
+	     *ok = false;
+	 return 0;
+    }
+    char *end;
+    double val = kjs_strtod(Latin1Encoding().encode(characters(),
length()).data(), &end);
+    if (ok)
+	 *ok = end == 0 || *end == '\0';
TextEncoding::encode returns a CString which is destroyed after calling
kjs_strtod. This makes the end pointer dangling so *end could potentially
crash.

+    return val;
+}

I've uploaded a patch that doesn't break any layout tests.