‘mktime’ replacement on glibc systems

‘mktime’ replacement on glibc systems

[Ludovic Courtès]

Hello Paul,

Gnulib’s ‘mktime’ replacement gets used on glibc systems (glibc 2.22
here) due to ‘__mktime_internal’ being unavailable, even though
gl_cv_func_working_mktime=yes on these systems.  Is this intended?

Furthermore, lib/mktime.c has this:

--8<---------------cut here---------------start------------->8---
time_t
mktime (struct tm *tp)
{
#ifdef _LIBC
  /* POSIX.1 8.1.1 requires that whenever mktime() is called, the
     time zone names contained in the external variable 'tzname' shall
     be set as if the tzset() function had been called.  */
  __tzset ();
#endif

  return __mktime_internal (tp, __localtime_r, &localtime_offset);
}
--8<---------------cut here---------------end--------------->8---

I believe “#ifdef _LIBC” should be changed to something like:

--8<---------------cut here---------------start------------->8---
#ifdef _LIBC
  __tzset ();
#else
  tzset ();
#endif
--8<---------------cut here---------------end--------------->8---

otherwise the current ‘TZ’ variable value ends up being ignored (we
noticed this problem with Guile’s test suite.)

WDYT?

Thanks,
Ludo’.

Re: ‘mktime’ replacement on glibc systems

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]

On 06/30/2016 10:21 AM, Ludovic Courtès wrote:
> Gnulib’s ‘mktime’ replacement gets used on glibc systems (glibc 2.22
> here) due to ‘__mktime_internal’ being unavailable, even though
> gl_cv_func_working_mktime=yes on these systems.  Is this intended?

No, and I don't observe it on Fedora 23 x86-64, which uses glibc 2.22. I 
ran './gnulib-tool --create-testdir --dir foo mktime; cd foo; 
./configure; make' and the resulting build did not compile mktime.c.

Perhaps your package is using the mktime-internal module? That would 
explain why mktime.c is getting compiled for you. If not, could you 
investigate why your package is compiling mktime.c even when glibc 2.22 
is being used?

> I believe “#ifdef _LIBC” should be changed to something like:
>
> --8<---------------cut here---------------start------------->8---
> #ifdef _LIBC
>    __tzset ();
> #else
>    tzset ();
> #endif
> --8<---------------cut here---------------end--------------->8---
>
> otherwise the current ‘TZ’ variable value ends up being ignored (we
> noticed this problem with Guile’s test suite.)
>

Yes, thanks, the old code dated back to when the substitute mktime.c was 
not intended to be thread-safe and so used localtime instead of 
localtime_r, so there was no need to call tzset. I installed the 
attached patch to fix that.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-mktime-call-tzset-as-per-POSIX.patch --]
[-- Type: text/x-patch; name="0001-mktime-call-tzset-as-per-POSIX.patch", Size: 2251 bytes --]

From 913c424ef1a10bacf63597ca7565b29908d6135b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 3 Jul 2016 12:59:54 +0200
Subject: [PATCH] mktime: call tzset as per POSIX
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Ludovic Courtès in:
http://lists.gnu.org/archive/html/bug-gnulib/2016-06/msg00068.html
* lib/mktime.c (mktime) [!_LIBC && HAVE_TZSET]: Call tzset.
* m4/mktime.m4 (gl_FUNC_MKTIME): Check for tzset.
---
 ChangeLog    | 8 ++++++++
 lib/mktime.c | 2 ++
 m4/mktime.m4 | 3 ++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 34b651f..ef33268 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2016-07-03  Paul Eggert  <eggert@cs.ucla.edu>
+
+	mktime: call tzset as per POSIX
+	Problem reported by Ludovic Courtès in:
+	http://lists.gnu.org/archive/html/bug-gnulib/2016-06/msg00068.html
+	* lib/mktime.c (mktime) [!_LIBC && HAVE_TZSET]: Call tzset.
+	* m4/mktime.m4 (gl_FUNC_MKTIME): Check for tzset.
+
 2016-06-26  Pádraig Brady  <P@draigBrady.com>
 
 	fts: handle readdir() errors
diff --git a/lib/mktime.c b/lib/mktime.c
index 87ab633..9eb3e76 100644
--- a/lib/mktime.c
+++ b/lib/mktime.c
@@ -470,6 +470,8 @@ mktime (struct tm *tp)
      time zone names contained in the external variable 'tzname' shall
      be set as if the tzset() function had been called.  */
   __tzset ();
+#elif HAVE_TZSET
+  tzset ();
 #endif
 
   return __mktime_internal (tp, __localtime_r, &localtime_offset);
diff --git a/m4/mktime.m4 b/m4/mktime.m4
index 5a0f2d8..23cad73 100644
--- a/m4/mktime.m4
+++ b/m4/mktime.m4
@@ -1,4 +1,4 @@
-# serial 26
+# serial 27
 dnl Copyright (C) 2002-2003, 2005-2007, 2009-2016 Free Software Foundation,
 dnl Inc.
 dnl This file is free software; the Free Software Foundation
@@ -30,6 +30,7 @@ AC_DEFUN([gl_FUNC_MKTIME],
   dnl in Autoconf and because it invokes AC_LIBOBJ.
   AC_CHECK_HEADERS_ONCE([unistd.h])
   AC_CHECK_DECLS_ONCE([alarm])
+  AC_CHECK_FUNCS_ONCE([tzset])
   AC_REQUIRE([gl_MULTIARCH])
   if test $APPLE_UNIVERSAL_BUILD = 1; then
     # A universal build on Apple Mac OS X platforms.
-- 
2.5.5

Re: ‘mktime’ replacement on glibc systems

[Ludovic Courtès]

Hi,

Paul Eggert <eggert@cs.ucla.edu> skribis:

> On 06/30/2016 10:21 AM, Ludovic Courtès wrote:
>> Gnulib’s ‘mktime’ replacement gets used on glibc systems (glibc 2.22
>> here) due to ‘__mktime_internal’ being unavailable, even though
>> gl_cv_func_working_mktime=yes on these systems.  Is this intended?
>
> No, and I don't observe it on Fedora 23 x86-64, which uses glibc
> 2.22. I ran './gnulib-tool --create-testdir --dir foo mktime; cd foo;
> ./configure; make' and the resulting build did not compile mktime.c.
>
> Perhaps your package is using the mktime-internal module? That would
> explain why mktime.c is getting compiled for you. If not, could you
> investigate why your package is compiling mktime.c even when glibc
> 2.22 is being used?

Our code indirectly pulls ‘timegm’, which has:

--8<---------------cut here---------------start------------->8---
Depends-on:
time
mktime-internal [test $HAVE_TIMEGM = 0 || test $REPLACE_TIMEGM = 1]
time_r          [test $HAVE_TIMEGM = 0 || test $REPLACE_TIMEGM = 1]
--8<---------------cut here---------------end--------------->8---

I guess the conditional is meant to avoid triggering the dependency.
However, in spite of:

--8<---------------cut here---------------start------------->8---
$ grep -E '(HAVE|REPLACE)_TIMEGM=' config.log
HAVE_TIMEGM='1'
REPLACE_TIMEGM='0'
--8<---------------cut here---------------end--------------->8---

… ‘gl_FUNC_MKTIME_INTERNAL’ is expanded and ends up setting
REPLACE_MKTIME=1.  IOW, the conditional does not prevent
mktime-internal’s configure snippet from being run.

Any idea how to address it?

>> I believe “#ifdef _LIBC” should be changed to something like:
>>
>> --8<---------------cut here---------------start------------->8---
>> #ifdef _LIBC
>>    __tzset ();
>> #else
>>    tzset ();
>> #endif
>> --8<---------------cut here---------------end--------------->8---
>>
>> otherwise the current ‘TZ’ variable value ends up being ignored (we
>> noticed this problem with Guile’s test suite.)
>>
>
> Yes, thanks, the old code dated back to when the substitute mktime.c
> was not intended to be thread-safe and so used localtime instead of
> localtime_r, so there was no need to call tzset. I installed the
> attached patch to fix that.

Great, thank you!

Ludo’.

Re: ‘mktime’ replacement on glibc systems

[Paul Eggert]

On 07/04/2016 09:53 AM, Ludovic Courtès wrote:
> the conditional does not prevent
> mktime-internal’s configure snippet from being run.
>
> Any idea how to address it?
Perhaps your bootstrap script is calling gnulib-tool without the 
--conditional-dependencies option? If so, you might try adding it. But 
please see the Gnulib manual's discussion of this option, a discussion 
that uses timegm as its example (I vaguely recall that timegm was the 
motivation for conditional dependencies...):

https://www.gnu.org/software/gnulib/manual/html_node/Conditional-dependencies.html

The Emacs build uses --conditional-dependencies; the Coreutils build 
does not, since it also uses --with-tests.

Re: ‘mktime’ replacement on glibc systems

[Ludovic Courtès]

Paul Eggert <eggert@cs.ucla.edu> skribis:

> On 07/04/2016 09:53 AM, Ludovic Courtès wrote:
>> the conditional does not prevent
>> mktime-internal’s configure snippet from being run.
>>
>> Any idea how to address it?
> Perhaps your bootstrap script is calling gnulib-tool without the
> --conditional-dependencies option? If so, you might try adding it. But
> please see the Gnulib manual's discussion of this option, a discussion
> that uses timegm as its example (I vaguely recall that timegm was the
> motivation for conditional dependencies...):
>
> https://www.gnu.org/software/gnulib/manual/html_node/Conditional-dependencies.html

Indeed, I had overlooked this feature.  Works for us, thank you!

Ludo’.