Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3382)

Issue 186168: [Issue 7632] Incorrect _Py_dg_strtod result for long strings (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 2 months ago by dickinsm
Modified:
14 years ago
Reviewers:
eric2
Base URL:
http://svn.python.org/view/*checkout*/python/trunk/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : With this fix, dp0, dp1 and dplen are no longer needed; remove them. #

Total comments: 9
Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -15 lines) Patch
M Lib/test/test_strtod.py View 1 chunk +2 lines, -0 lines 0 comments Download
M Python/dtoa.c View 1 6 chunks +41 lines, -15 lines 9 comments Download

Messages

Total messages: 2
eric2
I have a few minor comments, but given the constraints of working with the original ...
14 years, 2 months ago (2010-01-15 22:41:28 UTC) #1
dickinsm
14 years, 2 months ago (2010-01-16 08:34:31 UTC) #2
http://codereview.appspot.com/186168/diff/1003/1004
File Python/dtoa.c (right):

http://codereview.appspot.com/186168/diff/1003/1004#newcode1309
Python/dtoa.c:1309: if (i < nd - 1)
The next line assumes that at least one of the digits beyond this point is
nonzero, which needn't be true.  If all digits are zero, dd should be 0 rather
than 1.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1325
Python/dtoa.c:1325: if (i < nd - 1)
Ditto for the next line.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1479
Python/dtoa.c:1479: * s0, nd0, nd, e, y and z such that:
On 2010/01/15 22:41:28, eric2 wrote:
> it
> would be much easier to analyze this if the code before this point was in a
> separate function that communicated with this code only through these
> parameters.

Agreed.  I am interested in doing some more serious refactoring, but I'll try to
fix the known bugs non-invasively first.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1493
Python/dtoa.c:1493: *    currently assumes that it's nonzero; this is a bug)
In this file.  I've flagged those places with comments.

The bugs are already filed: they're bug 5 and bug 7 in the tracker.  The fix I'm
working on involves rewriting the parser so that nd never takes trailing zeros
into account.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1627
Python/dtoa.c:1627: if ((i < nd0 ? s0[i] : s0[i+1]) != '0') {
Will do.  Thanks.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1793
Python/dtoa.c:1793: i = -1; /* Discarded digits make delta smaller. */
This code assumes that at least one of the discarded digits is nonzero, which
needn't be true.  If all discarded digits are zero and i == 0, then i should be
left unchanged here.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b