use of "localtime" in FormatMagickTime not thread-safe

[egb]

The FormatMagickTime function in magick/string.c uses the non-reentrant libc-functions "localtime" and "gmtime".
So its possible multiple Threads are using the same "local_time" and "utc_time" structs, and one of them will format a wrong time.

glibc (POSIX.1-2001?) offers "localtime_r" and "gmtime_r" as reentrant replacement.

untested patch:

Code: Select all

MagickExport long FormatMagickTime(const time_t time,const size_t length,
  char *timestamp)
{
  long
    count;

#ifdef HAVE_LOCALTIME_R
  struct tm
     local_time_storage,
     utc_time_storage;
#endif

  struct tm
    *local_time,
    *utc_time;

  time_t
    timezone;

  assert(timestamp != (char *) NULL);
#ifdef HAVE_LOCALTIME_R
  local_time=localtime_r(&time,&local_time_storage);
#else
  local_time=localtime(&time);
#endif
  if (local_time == (struct tm *) NULL)
    return(0);
#ifdef HAVE_LOCALTIME_R
  utc_time=gmtime_r(&time,&utc_time_storage);
#else
  utc_time=gmtime(&time);
#endif
  if (utc_time == (struct tm *) NULL)
    return(0);
  timezone=(time_t) ((local_time->tm_min-utc_time->tm_min)/60+
    local_time->tm_hour-utc_time->tm_hour+24*((local_time->tm_year-
    utc_time->tm_year) != 0 ? (local_time->tm_year-utc_time->tm_year) :
    (local_time->tm_yday-utc_time->tm_yday)));
  count=FormatMagickString(timestamp,length,
    "%04d-%02d-%02dT%02d:%02d:%02d%+03ld:00",local_time->tm_year+1900,
    local_time->tm_mon+1,local_time->tm_mday,local_time->tm_hour,
    local_time->tm_min,local_time->tm_sec,(long) timezone);
  return(count);
}

[magick]

Thanks, we will get your patch into the ImageMagick Subversion trunk by sometime tomorrow.

[egb]

Thanks, we will get your patch into the ImageMagick Subversion trunk by sometime tomorrow.

Great.
Keep in mind you'll have to add "localtime_r" to the "AC_CHECK_FUNCS"-call in configure.ac to acutally get the HAVE_LOCALTIME_R define in config.h

[magick]

Right, the patch to string.c and configure.ac are already in the Subversion trunk. Thanks,