[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





------- Comment #13 from cwzwarich at uwaterloo.ca  2007-07-17 00:05 PDT -------
(In reply to comment #12)
> 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.

Oops. Fixed!

> 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.

I took the usage of HUGE_VAL from existing code, so I figured it was okay. I'll
change it to Inf.

> +          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.

The code is there because we are passing off the conversion to strtod().
However, strtod() will interpret anything like INF, inf, etc. as IEEE infinity.
However, the ECMA spec says that only "Infinity" is infinity, so we if strtod()
returns infinity we need to check that it was not for the wrong reason, as if
it was "inf" we should return NaN as per the spec.

> 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.

The s[p] check (in the original version where I copied the string) actually
works and fixes a case. Without it,

parseInt("9007199254740992e2000")

returns Infinity. However, the check for the decimal point will never actually
affect anything besides possibly the running time of strtod(), because if we've
gone past the 53-bit range of the mantissa before the decimal point then
nothing after it will affect the value. I figured I might as well add it in
since I am checking for 'e'.

I'll add a LayoutTest with examples for all of the branches in my code beyond
the cases in the Mozilla tests and then upload a new diff.


-- 
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.