Re: [Help-glpk] things I don't understand in src/env/time.c + bugs + suggested patch

[Andrew Makhorin]

> Over glpk-4.61
> 
> src/env/time.c contains:
> 
> double glp_time(void)
> {     struct timeval tv;
>       struct tm *tm;
>       int j;
>       double t;
>       gettimeofday(&tv, NULL);
>       tm = gmtime(&tv.tv_sec);
>       j = jday(tm->tm_mday, tm->tm_mon + 1, 1900 + tm->tm_year);
>       xassert(j >= 0);
>       t = ((((double)(j - EPOCH) * 24.0 + (double)tm->tm_hour) * 60.0 +
>          (double)tm->tm_min) * 60.0 + (double)tm->tm_sec) * 1000.0 +
>          (double)(tv.tv_usec / 1000);
>       return t;
> }

This function is a patched version of the ANSI C version following below
in the same file.

> 
> This code suffers from several problems:
> - The division tv.tv_usec / 1000 is integer division, then converted to
> double; why not simply multiply by 1E-3, which is more precise?

Integer arithmetic is used to avoid a well-known undesirable effect when
a floating-point result is returned in a register which provides wider
precision than double. See
http://cygwin.com/ml/cygwin/2007-02/msg00386.html

> - It uses gmtime, which is not reentrant (as opposed to gmtime_r). 

It depends on the implementation. For example, the static object, a
pointer to which is returned, may have a thread local storage specifier.

> In
> fact, my attention was called to this function because of error reports
> by Valgrind's DRD race condition analyser ran on code using glpk from
> different threads.
> 
> I also do not understand why it is useful to use such a function,
> calling jday() to convert Julian days, whereas the following returns the
> same result:
> 
> double glp_time(void)
> {     struct timeval tv;
>        gettimeofday(&tv, NULL);
>       return tv.tv_sec + 1E3 * tv.tv_usec * 1E-3;
> }
> 

The Standard (POSIX) does not specify the epoch (which thus may differ
from 01/01/1970), so gmtime is used.

> What am I missing?

> If I'm missing nothing, this patch simplifies the code:
> 
> --- glpk-4.61-orig/src/env/time.c    2017-01-22 08:00:00.000000000 +0100
> +++ glpk-4.61-time-patched/src/env/time.c    2017-01-28
> 13:09:07.367464607 +0100
> @@ -53,17 +53,8 @@
>  
>  double glp_time(void)
>  {     struct timeval tv;
> -      struct tm *tm;
> -      int j;
> -      double t;
>        gettimeofday(&tv, NULL);
> -      tm = gmtime(&tv.tv_sec);
> -      j = jday(tm->tm_mday, tm->tm_mon + 1, 1900 + tm->tm_year);
> -      xassert(j >= 0);
> -      t = ((((double)(j - EPOCH) * 24.0 + (double)tm->tm_hour) * 60.0 +
> -         (double)tm->tm_min) * 60.0 + (double)tm->tm_sec) * 1000.0 +
> -         (double)(tv.tv_usec / 1000);
> -      return t;
> +      return tv.tv_sec + 1E3 * tv.tv_usec * 1E-3;
>  }
>  
>  /* MS Windows version *************************************************/
> @@ -92,17 +83,7 @@
>  #include <time.h>
>  
>  double glp_time(void)
> -{     time_t timer;
> -      struct tm *tm;
> -      int j;
> -      double t;
> -      timer = time(NULL);
> -      tm = gmtime(&timer);
> -      j = jday(tm->tm_mday, tm->tm_mon + 1, 1900 + tm->tm_year);
> -      xassert(j >= 0);
> -      t = ((((double)(j - EPOCH) * 24.0 + (double)tm->tm_hour) * 60.0 +
> -         (double)tm->tm_min) * 60.0 + (double)tm->tm_sec) * 1000.0;
> -      return t;
> +{      return time(NULL) * 1E3;
>  }
>  
>  #endif
> 

Thank you for the patch. For reasons mentioned above I'd like to keep
the current version; however, I will replace gmtime with gmtime_r.

Best regards,

Andrew Makhorin