[gnuastro-commits] master c11846f5: Library (units.h): using signbit() instead of <0.0f

[Mohammad Akhlaghi]

branch: master
commit c11846f5af9e205c102896076014666f3a1cb347
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (units.h): using signbit() instead of <0.0f
    
    Until now, while parsing the sexagesimal RA or Dec strings, to check for
    negative values, we were simply using '<0.0f'. However, this test will fail
    when given a negative zero (or '-0.0', which can happen in sexagesimal
    coordiantes in small declinations for example '-00d12m34'). As a result, a
    command like below would effectively ignore the sign in its output:
    
        $ echo -00h12m34 | asttable
        3.141666667
    
    With this commit, the two 'gal_units_ra_to_degree' and
    'gal_units_dec_to_degree' functions now use the 'signbit()' function of the
    C library to check for the "sign" bit (and thus identify if it is positive
    or negative). This allows them to properly detect a negative zero and
    account for it like all negative numbers.
    
    This bug was reported by Manuel Sánchez-Benavente.
    
    This fixes bug #62784.
---
 NEWS           |  3 +++
 bootstrap.conf |  1 +
 lib/txt.c      |  9 +++++----
 lib/units.c    | 29 +++++++++++++++++++++--------
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index cdeccb2c..381a4259 100644
--- a/NEWS
+++ b/NEWS
@@ -230,6 +230,9 @@ See the end of the file for license conditions.
               tables. Found by Hilderic Browne.
   bug #62720: Table not reading string columns that are shorter than the
               defined width in the metadata.
+  bug #62784: Conversion of sexagesimal RA or Dec ignores sign when first
+              digit is zero (for example '-00d12m34'). Found by Manuel
+              Sánchez-Benavente.
 
 
 
diff --git a/bootstrap.conf b/bootstrap.conf
index be0951de..891f077b 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -206,6 +206,7 @@ gnulib_modules="
     stdint
     strtod
     mktime
+    signbit
     fcntl-h
     havelib
     memmove
diff --git a/lib/txt.c b/lib/txt.c
index f102d1eb..7628efab 100644
--- a/lib/txt.c
+++ b/lib/txt.c
@@ -427,14 +427,15 @@ txt_info_from_first_row(char *in_line, gal_data_t 
**datall, int format,
               if( gal_type_from_string( &tmpdptr, token, GAL_TYPE_FLOAT64)
                   && isnan( gal_units_ra_to_degree(token) )
                   && isnan( gal_units_dec_to_degree(token) ) )
-                error(EXIT_FAILURE, 0, "'%s' couldn't be read as a number "
-                      "(element %zu of first uncommented line) %f ", token,
-                      ncol, gal_units_ra_to_degree(token));
+                error(EXIT_FAILURE, 0, "'%s' couldn't be read as a "
+                      "number (element %zu of first uncommented line)",
+                      token, ncol);
 
               /* Allocate this column's dataset and set it's 'status' to
                  the column number that it corresponds to. */
               gal_list_data_add_alloc(datall, NULL, GAL_TYPE_FLOAT64, 0,
-                                      NULL, NULL, 0, -1, 1, NULL, NULL, NULL);
+                                      NULL, NULL, 0, -1, 1, NULL, NULL,
+                                      NULL);
               (*datall)->status=ncol;
             }
         }
diff --git a/lib/units.c b/lib/units.c
index c6e243d6..0ee082b6 100644
--- a/lib/units.c
+++ b/lib/units.c
@@ -130,16 +130,22 @@ gal_units_ra_to_degree(char *convert)
   /* Check whether the string is successfully parsed */
   if(gal_units_extract_decimal(convert, ":hms", val, 3))
     {
-      /* Check whether the first value is in within limits, and add it. */
-      if(val[0]<0.0 || val[0]>24.0) return NAN;
+      /* Check whether the first value is in within limits, and add it. We
+         are using 'signbit(val[0])' instead of 'val[0]<0.0f' because
+         'val[0]<0.0f' can't distinguish negative zero (-0.0) from an
+         unsigned zero (in other words, '-0.0' will be interpretted to be
+         positive). For the declinations it is possible (see the comments
+         in 'gal_units_dec_to_degree'), so a user may mistakenly give that
+         format in Right Ascension. */
+      if(signbit(val[0]) || val[0]>24.0) return NAN;
       decimal += val[0];
 
       /* Check whether value of minutes is within limits, and add it. */
-      if(val[1]<0.0 || val[1]>60.0) return NAN;
+      if(signbit(val[1]) || val[1]>60.0) return NAN;
       decimal += val[1] / 60;
 
       /* Check whether value of seconds is in within limits, and add it. */
-      if(val[2]<0.0 || val[2]>60.0) return NAN;
+      if(signbit(val[2]) || val[2]>60.0) return NAN;
       decimal += val[2] / 3600;
 
       /* Convert value to degrees and return. */
@@ -177,16 +183,23 @@ gal_units_dec_to_degree(char *convert)
          negative and all other values will be positive. In that case, we
          set sign equal to -1. Therefore, we multiply the first value by
          sign to make it positive. The final answer is again multiplied by
-         sign to make its sign same as original. */
-      sign = val[0]<0.0 ? -1 : 1;
+         sign to make its sign same as original.
+
+        We are using 'signbit(val[0])' instead of 'val[0]<0.0f' because
+        'val[0]<0.0f' can't distinguish negative zero (-0.0) from an
+        unsigned zero (in other words, '-0.0' will be interpretted to be
+        positive). In the case of declination, this can happen just below
+        the equator (where the declination is less than one degree), for
+        example '-00d:12:34'*/
+      sign = signbit(val[0]) ? -1 : 1;
       decimal += val[0] * sign;
 
       /* Check whether value of arc-minutes is in within limits. */
-      if(val[1]<0.0 || val[1]>60.0) return NAN;
+      if(signbit(val[1]) || val[1]>60.0) return NAN;
       decimal += val[1] / 60;
 
       /* Check whether value of arc-seconds is in within limits */
-      if (val[2] < 0.0 || val[2] > 60.0) return NAN;
+      if (signbit(val[2]) || val[2] > 60.0) return NAN;
       decimal += val[2] / 3600;
 
       /* Make the sign of the decimal value same as input and return. */