Incorrect negative NAN behavior in cpl_strtod.cpp:CPLStrtodDelim

[Kurt Schwehr]

-1.#QNAN ends up giving back a NAN rather than -NAN. That seems to be wrong.

source:trunk/gdal/port/cpl_strtod.cpp#L265​

It could just do:

#include <limits>

// snip
return -std::numeric_limits<double>::quiet_NaN();

What are the implications of changing this?

    if (nptr[0] == '-')
    {
        if (strcmp(nptr, "-1.#QNAN") == 0 ||
            strcmp(nptr, "-1.#IND") == 0)
        {
            if( endptr ) *endptr = (char*)nptr + strlen(nptr);
            return NAN;
        }

to

    if (nptr[0] == '-')
    {
        if (strcmp(nptr, "-1.#QNAN") == 0 ||
            strcmp(nptr, "-1.#IND") == 0)
        {
            if( endptr ) *endptr = (char*)nptr + strlen(nptr);
            return -std::numeric_limits<double>::quiet_NaN();
        }

[Kurt Schwehr]

Here is the proposed patch. Will this work with MSVC compilers, Android etc?

cpl_strtod.cpp

@@ -29 +29 @@
  ****************************************************************************/
 
 #include <locale.h>
-#include <errno.h>
-#include <stdlib.h>
 
+#include <cerrno>
+#include <cstdlib>
+#include <limits>
+
 #include "cpl_conv.h"
 
 CPL_CVSID("$Id$");
@@ -44 +46 @@
 }
 #endif
 
-#ifndef NAN
-#  ifdef HUGE_VAL
-#    define NAN (HUGE_VAL * 0.0)
-#  else
-
-static float CPLNaN(void)
-{
-    float fNan;
-    int nNan = 0x7FC00000;
-    memcpy(&fNan, &nNan, 4);
-    return fNan;
-}
-
-#    define NAN CPLNan()
-#  endif
-#endif
-
-#ifndef INFINITY
-    static CPL_INLINE double CPLInfinity(void)
-    {
-        static double ZERO = 0;
-        return 1.0 / ZERO; /* MSVC doesn't like 1.0 / 0.0 */
-    }
-    #define INFINITY CPLInfinity()
-    static CPL_INLINE double CPLNegInfinity(void)
-    {
-        static double ZERO = 0;
-        return -1.0 / ZERO; /* MSVC doesn't like -1.0 / 0.0 */
-    }
-    #define NEG_INFINITY CPLNegInfinity()
-#else
-    #define NEG_INFINITY (-INFINITY)
-#endif
-
 /************************************************************************/
 /*                            CPLAtofDelim()                            */
 /************************************************************************/
@@ -266 +234 @@
             strcmp(nptr, "-1.#IND") == 0)
         {
             if( endptr ) *endptr = (char*)nptr + strlen(nptr);
-            return NAN;
+            return -std::numeric_limits<double>::quiet_NaN();
         }
 
         if (strcmp(nptr,"-inf") == 0 ||
@@ -273 +241 @@
             EQUALN (nptr,"-1.#INF",strlen("-1.#INF")))
         {
             if( endptr ) *endptr = (char*)nptr + strlen(nptr);
-            return NEG_INFINITY;
+            return -std::numeric_limits<double>::infinity();
         }
     }
     else if (nptr[0] == '1')
@@ -281 +249 @@
         if (strcmp(nptr, "1.#QNAN") == 0)
         {
             if( endptr ) *endptr = (char*)nptr + strlen(nptr);
-            return NAN;
+            return std::numeric_limits<double>::quiet_NaN();
         }
         if( EQUALN (nptr,"1.#INF",strlen("1.#INF")) )
         {
             if( endptr ) *endptr = (char*)nptr + strlen(nptr);
-            return INFINITY;
+            return std::numeric_limits<double>::infinity();
         }
     }
     else if (nptr[0] == 'i' && strcmp(nptr,"inf") == 0)
     {
         if( endptr ) *endptr = (char*)nptr + strlen(nptr);
-        return INFINITY;
+        return std::numeric_limits<double>::infinity();
     }
     else if (nptr[0] == 'n' && strcmp(nptr,"nan") == 0)
     {
         if( endptr ) *endptr = (char*)nptr + strlen(nptr);
-        return NAN;
+        return std::numeric_limits<double>::quiet_NaN();
     }
 
 /* -------------------------------------------------------------------- */

[Kurt Schwehr]

Trying out limit and quiet_NaN via r30769

Note that I see comments to the effect of Tru64 support around the isnan. Do we need to support Tru64 for gdal >= 2.1.0 when support for the os was EOL'ed 3 years ago? If we support platforms with NaN and Inf issues, wht is the current list?

[Even Rouault]

Note that I see comments to the effect of Tru64 support around the isnan. Do we need to support Tru64 for gdal >= 2.1.0 when support for the os was EOL'ed 3 years ago?

Probably not. I've hardly heard about that platform before

If we support platforms with NaN and Inf issues, wht is the current list?

No idea... GDAL at least compiles on most Debian architectures ( ​https://packages.debian.org/wheezy/gdal-bin ), not sure about the runtime correctness however. Plus some BSD's. Plus Android.

[Kurt Schwehr]

Reapplied argdataset.cpp nan test as r30781. It appears to work in travis-ci runs.

[Kurt Schwehr]

IEEE 754 draft: ​http://754r.ucbtest.org/drafts/archive/2006-10-04.pdf

[Kurt Schwehr]

Going to call this done. I think the current behavior is fine now that we have the std:: way of specifying NaNs and Inf.