Opened 17 years ago

Last modified 17 years ago

#1464 closed defect (fixed)

CPLAtof() gives wrong results

Reported by: warmerdam Owned by: warmerdam
Priority: highest Milestone: 1.4.1
Component: default Version: 1.4.0
Severity: blocker Keywords:
Cc:

Description (last modified by hobu)

This program:

#include "cpl_conv.h"

int main()

{
    printf( "%g\n", CPLAtof( "0.9996" ) );
}

Produces the result: 0.90996.

This becomes quite a pressing issue as we move to using this function
widely in the code.

Change History (6)

comment:1 by warmerdam, 17 years ago

Andrey,

Any thoughts on this?  I'm starting to have some doubts about the cpl_strod code

comment:2 by Mateusz Łoskot, 17 years ago

(In reply to comment #1)
> 
> Any thoughts on this?

I suppose the bug is in the digits recognition algorithm, in _Stold() function.
In for loop (lines 384:427), every time, after the first digit of decimal part is recognized (lines 406:407), the olead flag gets 1.
After this, there is no place in the algorithm where olead is reseting with new value, so the condition in line 406 is false.

In example:

0.1196

Iteration 1
-----------

else if (*sc == '0')

- condition in line 394 is true

Iteration 2
-----------

if (*sc == point)

- condition is true so the decimal point has been found

Iteration 3
-----------

if (olead < 0)
  olead = nzero;

- condition from line 406 is true and olead is set with 'saw the zero' flag, 1

Iteration 4
-----------

Again, the condition in line 406 is tested, but it's false now,
because olead == 1, what means there is some zero found, so the 'else' part is executed to fill buffer with zero(s).


Generally, this bug is interesting because it's relate to "what stands just after the decimal point". Here is a short test:

CPLAtof( "0.123" ) == 0.1023
CPLAtof( "0.0123" ) == 0.010023
CPLAtof( "0.00123" ) == 0.00100023
CPLAtof( "0.000123" ) == 0.000100002

I think it proofs there is a problem with 'seen zero'.
If there is NO zero in decimal part, the flag says 1 zero was seen.
If there is 1 zero -//-, the flag says 2 zeros
If there are 2 zeros -//-, the flag says 3 zeros
etc.

comment:3 by warmerdam, 17 years ago

I have modified CPLStrodDelim() to avoid the _Stold() call when the 
delimeter is ".".  This fixes most impact of this bug for now. 

  // Temporary hack till _Stold() is fixed per #1464 is fixed.
  if( point == '.' )
  {
      CPLLocaleC oLocaleEnforcer;
      
      return strtod( nptr, endptr );
  }


This change cannot stay permanently because we don't want to use locale
override, and because it doesn't help when processing "," as the delimiter.

Please comment out the above code when working on this bug.

comment:4 by dron, 17 years ago

Mateusz,

Thanks for your analisys, it was helpful! I fixed this bug and this is the test case (BTW, I am thinking how to test the 'port' stuff in our test suite):

#include <cpl_conv.h>

struct test_pairs
  {
    const char *strval;
    double numval;
  };

static const struct test_pairs tests[] =
  {
    { "12.345", 12.345 },
    { "12.345e19", 12.345e19 },
    { "-.1e+9", -.1e+9 },
    { ".123", .123 },
    { ".0123", .0123 },
    { ".00123", .00123 },
    { ".010203", .010203 },
    { ".01002003", .01002003 },
    { ".01002003", .01002003 },
    { "0.123", 0.123 },
    { "0.0123", 0.0123 },
    { "0.00123", 0.00123 },
    { "0.010203", 0.010203 },
    { "0.01002003", 0.01002003 },
    { "0.01002003", 0.01002003 },
    { "1.123", 1.123 },
    { "1.0123", 1.0123 },
    { "1.00123", 1.00123 },
    { "1.010203", 1.010203 },
    { "1.01002003", 1.01002003 },
    { "1.01002003", 1.01002003 },
    { "9.123", 9.123 },
    { "9.0123", 9.0123 },
    { "9.00123", 9.00123 },
    { "9.010203", 9.010203 },
    { "9.01002003", 9.01002003 },
    { "9.01002003", 9.01002003 },
    { "01.234", 1.234 },
    { "001.234", 1.234 },
    { "0001.234", 1.234 },
    { "00.123", 0.123 },
    { "000.123", 0.123 },
    { "0000.123", 0.123 },
    { "1e20", 1e20 },
    { "0e-19", 0 },
    { "4\00012", 4.0 },
    { "5.9e-76", 5.9e-76 },
    { NULL, 0 }
  };

int main()

{
    static const struct test_pairs  *test = tests;
    while (test->strval)
    {
        printf("Got %g, expected %g\n", CPLAtof(test->strval), test->numval);
        test++;
    }

    return 0;
}


comment:5 by warmerdam, 17 years ago

Confirmed ... Thanks Andrey!

I'd note that the fix went into 1.4 branch as well as trunk which is good.

comment:10 by hobu, 17 years ago

Description: modified (diff)
Milestone: 1.4.1
Note: See TracTickets for help on using tickets.