[Webkit-unassigned] [Bug 9697] parseInt results may be inaccurate for numbers greater than 2^53

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=9697


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #15528|review?                     |review-
               Flag|                            |




------- Comment #12 from darin at apple.com  2007-07-16 22:17 PDT -------
(From update of attachment 15528)
Looking great! I love that this fixes so many things in existing tests. I still
see a few problems.

The variable "p" should be declared as a const char* -- then you wouldn't need
the typecast on this line:

+    for (p = ((char*) s) + length - 1; p >= s; p--) {

I don't see any use of p that would require a non-const pointer.

I know Adam suggested this:

+                s[p] = '\0';

but it won't work -- you can't modify a const UString&. In fact, if it
compiles, we should probably fix this line from the ustring.h header:

    UChar operator[](int pos) const;

If we change it to return a const UChar then you'd see a compiler error.

The right want to do this is simply to call s.substr(firstDigitPosition, p -
firstDigitPosition).ascii(). The UString class has an appropriate optimization,
so this won't have to allocate an entirely new string buffer.

In fact, the other callers should also call
s.substr(firstDigitPosition).ascii(), so the buffer created by the ascii()
function can be smaller.

Are you sure that HUGE_VAL works on all the platforms we need to compile on? I
ask because we have KJS::Inf in value.h and we wouldn't need it if we could
just use HUGE_VAL.

+          if (strncasecmp(c, "inf", 3) == 0)
+            return NaN;

I don't understand the code above at all, and I'd like to see the test cases
that demonstrate we need it and why it's OK to just look at the first three
characters.

Because this code was written incorrectly but the tests still passed, I think
we need to devise at least one more test to ensure that everything is working
correctly. If you can't find a test that fails due to the s[p] mistake, then I
suspect that you don't need to do the truncation at all -- I suspect kjs_strtod
will stop just fine when it encounters an "e" or a "." and if not we need a
test to demonstrate it working. In general we need a test that exercises every
branch in the new code, and that's not true at the moment.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.