bugGNU Astronomy Utilities - Bugs: bug #58455, Timezone calculation is not...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

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

Submitter:  Mohammad Akhlaghi <makhlaghi>
Submitted:  Wed 27 May 2020 04:08:16 PM UTC
   
 
Category:  Libraries Severity:  3 - Normal
Item Group:  Crash Status:  Fixed
Privacy:  Public Assigned to:  makhlaghi
Open/Closed:  Closed

Wed 27 May 2020 09:24:43 PM UTC, comment #2: 

This bug has been implemented and pushed to the main repository.

Thanks again Alexey for reporting this bug. The problem was infact more serious: the 'timezone' was not used for the current time, but for a different time (in the headers of datasets, usually the time the datasets were created).

While I was looking into the fix, I noticed that by simply subtracting 'timezone', the result of the same command would be different in different seasons (when daylight-saving was active, there would a one-hour difference!). But I was impressed that 'mktime' actually calculates the timezone of the local system at the time that 'tp' points to, thus correcting this error.

So I am closing this bug, but it would be great if you could give it a final try on FreeBSD and see if there is no more building problem. You can get the most recent tarball (that now has this bug fixed) from this URL: https://akhlaghi.org/gnuastro-latest.tar.lz

Mohammad Akhlaghi <makhlaghi>
Group administrator
Wed 27 May 2020 04:11:27 PM UTC, comment #1: 

I forgot to put the code piece of the original bug-report, I am putting it here.


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;


Mohammad Akhlaghi <makhlaghi>
Group administrator
Wed 27 May 2020 04:08:16 PM UTC, original submission:  

This bug was initially submitted to the bug-gnuastro@gnu.org 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 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.

Mohammad Akhlaghi <makhlaghi>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by makhlaghi
  • -email is unavailable- added by makhlaghi
  • -email is unavailable- added by makhlaghi (Submitted the item)
  • -email is unavailable- added by makhlaghi
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-05-27 makhlaghi StatusConfirmed Fixed
        Open/ClosedOpen Closed
        Carbon-Copy- Added -email is unavailable-
        Carbon-Copy- Added -email is unavailable-
    2020-05-27 makhlaghi Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-b921.
    Corresponding source code