[gnuastro-commits] master 2cf0801: Library (fits): Timezone correctly used and not subtracting flag

[Mohammad Akhlaghi]

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

    Library (fits): Timezone correctly used and not subtracting flag
    
    Until now, the function 'gal_fits_key_date_to_seconds' (which was used by
    the Fits program's '--datetosec' option, which is used in
    'astscript-sort-by-night') would incorrectly subtract the current system
    timezone from the output of 'mktime'. But this doesn't take into effect
    that the time in the FITS header may not have the same daylight saving as
    the run-time.
    
    With this commit, instead of subtracting the local timezone through the
    global variable 'timezone', we are subtracting the timezone at the time of
    the input.
    
    Also, we were incorrectly subtracting the global variable 'daylight' from
    the resulting time!!! This was wrong: 'daylight' is a flag: only having
    values of 0 or 1. sSo all times reported until now, may have been off by
    one second (depending on the system's timezone!).
    
    Finally, we weren't actually checking until now if 'mktime' has succeeded
    or not! With this commit, if it doesn't (and returns '-1'),
    'gal_fits_key_date_to_seconds' will return 'GAL_BLANK_SIZE_T', and the Fits
    program (which is currently the only program calling this function) will
    print an informative error.
    
    This bug was found by Alexey Dokuchaev.
    
    This fixes bug #58455.
---
 NEWS                         |  3 ++-
 bin/fits/keywords.c          |  2 ++
 doc/announce-acknowledge.txt |  1 +
 doc/gnuastro.texi            |  1 +
 lib/fits.c                   | 26 ++++++++++++++++++--------
 5 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/NEWS b/NEWS
index efca08b..fb0a459 100644
--- a/NEWS
+++ b/NEWS
@@ -28,7 +28,8 @@ See the end of the file for license conditions.
      floating point if they had non-zero decimals.
 
 ** Bugs fixed
-  bug #58434: MakeCatalog crash when ordering is required and no usable pixels
+  bug #58434: MakeCatalog crash when ordering is required and no usable pixels.
+  bug #58455: Timezone is not portable and uses flag instead of seconds.
 
 
 
diff --git a/bin/fits/keywords.c b/bin/fits/keywords.c
index 59a8070..239fc5f 100644
--- a/bin/fits/keywords.c
+++ b/bin/fits/keywords.c
@@ -413,6 +413,8 @@ keywords_date_to_seconds(struct fitsparams *p, fitsfile 
*fptr)
   /* Return the number of seconds (and subseconds) that it corresponds
      to. */
   seconds=gal_fits_key_date_to_seconds(fitsdate, &subsecstr, &subsec);
+  if(seconds==GAL_BLANK_SIZE_T)
+    error(EXIT_FAILURE, 0, "the time string couldn't be interpretted");
 
   /* Print the result (for the sub-seconds, print everything after the */
   if( !p->cp.quiet )
diff --git a/doc/announce-acknowledge.txt b/doc/announce-acknowledge.txt
index 010cc61..b15ccb6 100644
--- a/doc/announce-acknowledge.txt
+++ b/doc/announce-acknowledge.txt
@@ -1,6 +1,7 @@
 Alphabetically ordered list to acknowledge in the next release.
 
 Carlos Allende Prieto
+Alexey Dokuchaev
 Raúl Infante Sainz
 Zahra Sharbaf
 Ole Streicher
diff --git a/doc/gnuastro.texi b/doc/gnuastro.texi
index f7aa34c..eb45d55 100644
--- a/doc/gnuastro.texi
+++ b/doc/gnuastro.texi
@@ -21356,6 +21356,7 @@ In this case (following the GNU C Library), this option 
will make the following
 @cindex Unix epoch time
 @cindex Epoch time, Unix
 Return the Unix epoch time (number of seconds that have passed since 00:00:00 
Thursday, January 1st, 1970) corresponding to the FITS date format string 
@code{fitsdate} (see description of @code{gal_fits_key_date_to_struct_tm} 
above).
+This function will return @code{GAL_BLANK_SIZE_T} if the broken-down time 
couldn't be converted to seconds.
 
 The Unix epoch time is in units of seconds, but the FITS date format allows 
sub-second accuracy.
 The last two arguments are for the (optional) sub-second portion.
diff --git a/lib/fits.c b/lib/fits.c
index 819119b..96682ba 100644
--- a/lib/fits.c
+++ b/lib/fits.c
@@ -1023,6 +1023,7 @@ gal_fits_key_date_to_seconds(char *fitsdate, char 
**subsecstr,
   time_t t;
   char *tmp;
   struct tm tp;
+  size_t seconds;
   void *outptr=subsec;
 
   /* Fill in the 'tp' elements with values read from the string. */
@@ -1044,14 +1045,23 @@ gal_fits_key_date_to_seconds(char *fitsdate, char 
**subsecstr,
                 tmp);
     }
 
-  /* Convert the 'tm' structure to 'time_t'. Note that the system's
-     timezone and daylight saving need to be subtracted from the output of
-     'mktime'. Otherwise the result will be different on different
-     host-system timezones (which is not what we want here: bug #57995). */
-  t=mktime(&tp)-timezone-daylight;
-
-  /* Return the value and set the output pointer. */
-  return (size_t)t;
+  /* Convert the contents of the 'tm' structure to 'time_t' (a positive
+     integer) with 'mktime'. Note that by design, the system's timezone is
+     included in the returned value of 'mktime' (leading to situations like
+     bug #57995). But it writes the given time's timezone (number of
+     seconds ahead of UTC) in the 'tm_gmtoff' element of its input.
+
+     IMPORTANT NOTE: the timezone that is calculated by 'mktime' (in
+     'tp.tm_gmtoff') belongs to the time that is already within 'tp' (this
+     is exactly what we want!). So for example when daylight saving is
+     activated at run-time, but at the time inside 'tp', there was no
+     daylight saving, the value of 'tp.tm_gmtoff' will be different from
+     the 'timezone' global variable. */
+  t=mktime(&tp);
+
+  /* Calculate the seconds and return it. */
+  seconds = (t == (time_t)(-1)) ? GAL_BLANK_SIZE_T : (t+tp.tm_gmtoff);
+  return seconds;
 }