[bug #58455] Timezone calculation is not portable and uses flag instead of seconds

[Mohammad Akhlaghi]

URL:
  <https://savannah.gnu.org/bugs/?58455>

                 Summary: Timezone calculation is not portable and uses flag
instead of seconds
                 Project: GNU Astronomy Utilities
            Submitted by: makhlaghi
            Submitted on: Wed 27 May 2020 05:08:16 PM BST
                Category: Libraries
                Severity: 3 - Normal
              Item Group: Crash
                  Status: Confirmed
                 Privacy: Public
             Assigned to: makhlaghi
             Open/Closed: Open
         Discussion Lock: Any

    _______________________________________________________

Details:

This bug was initially submitted to the address@hidden
<https://lists.gnu.org/archive/html/bug-gnuastro/2020-05/msg00007.html>
mailing list by Alexey Dokuchaev (thanks a lot Alexey! this is a great point).
I am defining it as a bug here to treat it more formally (and assign a number
to it).

--------

The problem is with bug #57995, or more precisely, with
Commit 51ee136
<http://git.savannah.gnu.org/cgit/gnuastro.git/commit/?id=51ee136> which
attempts to correct it. Actually, there are two problems with it. 

1. It does not look right to me that you're subtracting "daylight". As per the
comment in the <time.h> it is a flag if daylight-saving time is ever in use. 
I don't think it's correct to do the math on its value (true/false), e.g.,
subtract from time_t which is seconds). I believe you just need to subtract
the timezone (the UTC offset) as it already should include any required DST
adjustment, but I'm not an expert on those things.

2. Second, the code is not portable.  While X/Open XSI does specify both
global variables, implementations typically discourage the use of external
variables; the tm_zone entry in the tm structure is preferred.  Consider the
following pseudo-code which should work everywhere:


t = mktime(&tp);
/*
 * The function localtime() converts the time value pointed
 * by parameter, and returns pointer to a "struct tm" internal
 * static object which contains the information for the current
 * time zone and other factors such as Daylight Saving Time).
 */
t += localtime(&t)->tm_gmtoff;
+verbatim

Ideally, perhaps you should also check for mktime() return value (if the time
cannot be represented, it returns -1) but I guess you are aware of that and
know what you're going.




    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?58455>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/