Paul Eggert wrote:
> > After the Solaris crash fix, the nstrftime tests still fail, due to this
> > output:
> >
> > <-00>0: expected "1970-01-01 00:00:00 -0000 (-00)", got "1970-01-01
> > 00:00:00 +0000 (-00)"
> > <-00>0: expected "1985-11-05 00:53:21 -0000 (-00)", got "1985-11-05
> > 00:53:21 +0000 (-00)"
> > <-00>0: expected "2001-09-09 01:46:42 -0000 (-00)", got "2001-09-09
> > 01:46:42 +0000 (-00)"
> ...
> I installed the attached to try to fix that issue. Thanks for reporting it.
Thanks. I confirm it fixes the test failure on Solaris 11.4.
But there are two problems:
1) On FreeBSD 5.2.1 and OpenBSD 6.0, I see a compiler warning:
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../../gllib -I..
-DGNULIB_STRICT_CHECKING=1 -I/home/bruno/include -I/usr/local/include -Wall
-D_THREAD_SAFE -g -O2 -MT c-nstrftime.o -MD -MP -MF .deps/c-nstrftime.Tpo -c
-o c-nstrftime.o ../../gllib/c-nstrftime.c
In file included from ../../gllib/c-nstrftime.c:20:
../../gllib/strftime.c: In function `__strftime_internal':
../../gllib/strftime.c:1471: warning: label `underlying_strftime' defined but
not used
Also, there is an inconsistency: In underlying_strftime, the condition is
# if USE_C_LOCALE && (HAVE_STRFTIME_L || HAVE_STRFTIME_LZ)
whereas the rest of the code has
# if USE_C_LOCALE && HAVE_STRFTIME_L
and also a dozen of
# if USE_C_LOCALE && !HAVE_STRFTIME_L
Fixed through the first patch below.
2) On NetBSD, one out of 4 unit tests fails:
PASS: test-c-nstrftime-1.sh
PASS: test-c-nstrftime-2.sh
PASS: test-nstrftime-1.sh
FAIL: test-nstrftime-2.sh
The cause is a crash:
main
-> locales_test
-> FUNC_CHECKED
-> nstrftime
-> __strftime_internal
-> underlying_strftime
-> strftime_lz
-> _fmt
-> _fmt
-> __tzgetname50
tzgetname = __tzgetname50 assumes that its first argument is != NULL.
strftime_lz and _fmt pass their timezone_t argument down.
The crashing call is
underlying_strftime (tz=NULL, ubuf, ubufsize=1024, modifier=0,
format_char='c', tp)
tz=NULL means universal time (a.k.a. GMT) in our strftime.h, but is undefined
behaviour for NetBSD's functions. See https://man.netbsd.org/tzset.3 .
Fixed through the second patch below.
Bruno
>From 0e5d4c8bc02ea8615e43cae70a9fc114eabcf778 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Tue, 18 Jun 2024 14:27:18 +0200
Subject: [PATCH 1/3] c-nstrftime: Fix warning on platforms without strftime_l.
* lib/strftime.c: Add comment regarding HAVE_STRFTIME_L.
(underlying_strftime): Don't define if this function is not used.
Correct indentation. Simplify #if condition.
(__strftime_internal): Disable code that is not used on platforms
without strftime_l.
---
ChangeLog | 9 +++++++++
lib/strftime.c | 22 +++++++++++++++-------
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 75f06623d6..cb44449076 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2024-06-18 Bruno Haible <br...@clisp.org>
+
+ c-nstrftime: Fix warning on platforms without strftime_l.
+ * lib/strftime.c: Add comment regarding HAVE_STRFTIME_L.
+ (underlying_strftime): Don't define if this function is not used.
+ Correct indentation. Simplify #if condition.
+ (__strftime_internal): Disable code that is not used on platforms
+ without strftime_l.
+
2024-06-17 Paul Eggert <egg...@cs.ucla.edu>
Improve wording for Y2038 and largefile probes
diff --git a/lib/strftime.c b/lib/strftime.c
index ff52fc8119..a757b4f313 100644
--- a/lib/strftime.c
+++ b/lib/strftime.c
@@ -38,8 +38,6 @@
# include "time-internal.h"
#endif
-#define HAVE_NATIVE_TIME_Z (USE_C_LOCALE ? HAVE_STRFTIME_LZ : HAVE_STRFTIME_Z)
-
/* Whether to require GNU behavior for AM and PM indicators, even on
other platforms. This matters only in non-C locales.
The default is to require it; you can override this via
@@ -377,6 +375,15 @@ memcpy_uppcase (CHAR_T *dest, const CHAR_T *src, size_t len LOCALE_PARAM)
#endif
+/* Note: We assume that HAVE_STRFTIME_LZ implies HAVE_STRFTIME_L.
+ Otherwise, we would have to write (HAVE_STRFTIME_L || HAVE_STRFTIME_LZ)
+ instead of HAVE_STRFTIME_L everywhere. */
+
+/* Define to 1 if we can use the system's native functions that takes a
+ timezone_t argument. As of 2024, this is only true on NetBSD. */
+#define HAVE_NATIVE_TIME_Z \
+ (USE_C_LOCALE && HAVE_STRFTIME_L ? HAVE_STRFTIME_LZ : HAVE_STRFTIME_Z)
+
#if USE_C_LOCALE && HAVE_STRFTIME_L
/* Cache for the C locale object.
@@ -837,7 +844,8 @@ static size_t __strftime_internal (STREAM_OR_CHAR_T *, STRFTIME_ARG (size_t)
bool, enum pad_style, int, bool *
extra_args_spec LOCALE_PARAM);
-#ifndef _LIBC
+#if !defined _LIBC \
+ && (!(USE_C_LOCALE && !HAVE_STRFTIME_L) || !HAVE_STRUCT_TM_TM_ZONE)
/* Make sure we're calling the actual underlying strftime.
In some cases, time.h contains something like
@@ -869,17 +877,17 @@ underlying_strftime (timezone_t tz, char *ubuf, size_t ubufsize,
*u++ = format_char;
*u = '\0';
-#if !HAVE_NATIVE_TIME_Z
+# if !HAVE_NATIVE_TIME_Z
if (tz && tz != local_tz)
{
tz = set_tz (tz);
if (!tz)
return 0;
}
-#endif
+# endif
size_t len;
-# if USE_C_LOCALE && (HAVE_STRFTIME_L || HAVE_STRFTIME_LZ)
+# if USE_C_LOCALE && HAVE_STRFTIME_L
locale_t locale = c_locale ();
if (!locale)
return 0; /* errno is set here */
@@ -1467,7 +1475,7 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize)
}
break;
-#ifndef _LIBC
+#if !defined _LIBC && !(USE_C_LOCALE && !HAVE_STRFTIME_L)
underlying_strftime:
{
char ubuf[1024]; /* enough for any single format in practice */
--
2.34.1
>From de1ab2cd57af884c6b2389b7a24640c1f24e61ec Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Tue, 18 Jun 2024 14:58:30 +0200
Subject: [PATCH 2/3] nstrftime: Fix crash on NetBSD 10.0.
* lib/strftime.c (utc_timezone_cache): New variable.
(utc_timezone): New function.
(underlying_strftime): Use it when tz is NULL.
---
ChangeLog | 7 +++++++
lib/strftime.c | 28 ++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index cb44449076..0c39edd886 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-06-18 Bruno Haible <br...@clisp.org>
+
+ nstrftime: Fix crash on NetBSD 10.0.
+ * lib/strftime.c (utc_timezone_cache): New variable.
+ (utc_timezone): New function.
+ (underlying_strftime): Use it when tz is NULL.
+
2024-06-18 Bruno Haible <br...@clisp.org>
c-nstrftime: Fix warning on platforms without strftime_l.
diff --git a/lib/strftime.c b/lib/strftime.c
index a757b4f313..ffc1670f23 100644
--- a/lib/strftime.c
+++ b/lib/strftime.c
@@ -403,6 +403,25 @@ c_locale (void)
#endif
+#if HAVE_NATIVE_TIME_Z
+
+/* Cache for the UTC time zone object.
+ Marked volatile so that different threads see the same value
+ (avoids locking). */
+static volatile timezone_t utc_timezone_cache;
+
+/* Return the UTC time zone object, or (timezone_t) 0 with errno set
+ if it cannot be created. */
+static timezone_t
+utc_timezone (void)
+{
+ if (!utc_timezone_cache)
+ utc_timezone_cache = tzalloc ("UTC");
+ return utc_timezone_cache;
+}
+
+#endif
+
#if (defined __NetBSD__ || defined __sun) && REQUIRE_GNUISH_STRFTIME_AM_PM
@@ -877,6 +896,15 @@ underlying_strftime (timezone_t tz, char *ubuf, size_t ubufsize,
*u++ = format_char;
*u = '\0';
+# if HAVE_NATIVE_TIME_Z
+ if (!tz)
+ {
+ tz = utc_timezone ();
+ if (!tz)
+ return 0; /* errno is set here */
+ }
+# endif
+
# if !HAVE_NATIVE_TIME_Z
if (tz && tz != local_tz)
{
--
2.34.1
