Paul Eggert wrote:
> > On NetBSD, tzalloc() is in libc, and tzalloc("\\") returns NULL.
> > On other platforms, tzalloc() comes from Gnulib, and tzalloc("\\") returns
> > non-NULL.
> > 
> > Which behaviour is correct?
> 
> Both. The set of supported TZ values is system-dependent.

OK, then we need to
  - conditionally disable the respective parts of the parse-datetime test
    (done through patch 1 below),
  - update the documentation of tzalloc to clarify that it may return NULL
    for arguments that the implementation considers as invalid.
    (done through patch 2 below).

I also took the opportunity to move the documentation of the 'time_rz'
module from the module description to the .h file. Why?
  - Because the usual places where people look for reference documentation are
    the manual and the .h files. They don't look in the module description.
  - Because the reference documentation refers to arguments of these functions,
    by name. One cannot understand the documentation if the function
    declarations are far away.

Another thing: The module summary reads

   Reentrant time zone functions: localtime_rz, mktime_z, etc.

What does the word "reentrant" mean here? Since the functions localtime_rz,
mktime_z don't invoke themselves recursively with a different time zone
argument, nor do they take function pointer parameters (callback), I think
"reentrant" means nothing here.

A close term is "multithread-safe". The API could be implemented in a
multithread-safe way, but time_rz.c is not multithread-safe, due to the
function 'change_env'.

It is planned to provide a multithread-safe implementation at some point?

Bruno
>From 35f8ff2e1162bf3ee60d99b6812f2ae10f3f2898 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Sun, 14 Mar 2021 19:19:07 +0100
Subject: [PATCH 1/2] parse-datetime tests: Avoid a test failure on NetBSD.

Reported by Thomas Klausner <t...@giga.or.at> in
<https://lists.gnu.org/archive/html/bug-gnulib/2021-03/msg00069.html>.

* tests/test-parse-datetime.c (main): Skip two tests on NetBSD.
---
 ChangeLog                   | 7 +++++++
 tests/test-parse-datetime.c | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index a5ba848..63aaa7b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2021-03-14  Bruno Haible  <br...@clisp.org>
+
+	parse-datetime tests: Avoid a test failure on NetBSD.
+	Reported by Thomas Klausner <t...@giga.or.at> in
+	<https://lists.gnu.org/archive/html/bug-gnulib/2021-03/msg00069.html>.
+	* tests/test-parse-datetime.c (main): Skip two tests on NetBSD.
+
 2021-03-10  Paul Eggert  <egg...@cs.ucla.edu>
 
 	libc-config: port to DragonFlyBSD 5.9
diff --git a/tests/test-parse-datetime.c b/tests/test-parse-datetime.c
index acca47c..b972e96 100644
--- a/tests/test-parse-datetime.c
+++ b/tests/test-parse-datetime.c
@@ -431,8 +431,12 @@ main (int argc _GL_UNUSED, char **argv)
   ASSERT (   parse_datetime (&result, "TZ=\"\"", &now));
   ASSERT (   parse_datetime (&result, "TZ=\"\" ", &now));
   ASSERT (   parse_datetime (&result, " TZ=\"\"", &now));
+  /* Exercise patterns which may be valid or invalid, depending on the
+     platform.  */
+#if !defined __NetBSD__
   ASSERT (   parse_datetime (&result, "TZ=\"\\\\\"", &now));
   ASSERT (   parse_datetime (&result, "TZ=\"\\\"\"", &now));
+#endif
 
   /* Outlandishly-long time zone abbreviations should not cause problems.  */
   {
-- 
2.7.4

>From 02a474ef457f470776031b3cc38ee6e891dbff17 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Sun, 14 Mar 2021 19:22:07 +0100
Subject: [PATCH 2/2] time_rz: Put reference documentation into the .h file.

* lib/time.in.h (timezone_t, tzalloc, tzfree, localtime_rz, mktime_z):
Add comments, based on modules/time_rz.
* modules/time_rz (Comment): Remove section.
---
 ChangeLog       |  7 +++++++
 lib/time.in.h   | 42 ++++++++++++++++++++++++++++++++++++++++--
 modules/time_rz | 11 -----------
 3 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 63aaa7b..b71c327 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2021-03-14  Bruno Haible  <br...@clisp.org>
 
+	time_rz: Put reference documentation into the .h file.
+	* lib/time.in.h (timezone_t, tzalloc, tzfree, localtime_rz, mktime_z):
+	Add comments, based on modules/time_rz.
+	* modules/time_rz (Comment): Remove section.
+
+2021-03-14  Bruno Haible  <br...@clisp.org>
+
 	parse-datetime tests: Avoid a test failure on NetBSD.
 	Reported by Thomas Klausner <t...@giga.or.at> in
 	<https://lists.gnu.org/archive/html/bug-gnulib/2021-03/msg00069.html>.
diff --git a/lib/time.in.h b/lib/time.in.h
index 4da1172..44483d0 100644
--- a/lib/time.in.h
+++ b/lib/time.in.h
@@ -340,22 +340,60 @@ _GL_CXXALIASWARN (strftime);
 # endif
 
 # if defined _GNU_SOURCE && @GNULIB_TIME_RZ@ && ! @HAVE_TIMEZONE_T@
+/* Functions that use a first-class time zone data type, instead of
+   relying on an implicit global time zone.
+   Inspired by NetBSD.  */
+
+/* Represents a time zone.
+   (timezone_t) NULL stands for UTC.  */
 typedef struct tm_zone *timezone_t;
+
+/* tzalloc (name)
+   Returns a time zone object for the given time zone NAME.  This object
+   represents the time zone that other functions would use it the TZ
+   environment variable was set to NAME.
+   If NAME is NULL, the result represents the time zone that other functions
+   would use it the TZ environment variable was unset.
+   May return NULL if NAME is invalid (this is platform dependent) or
+   upon memory allocation failure.  */
 _GL_FUNCDECL_SYS (tzalloc, timezone_t, (char const *__name));
 _GL_CXXALIAS_SYS (tzalloc, timezone_t, (char const *__name));
+
+/* tzfree (tz)
+   Frees a time zone object.
+   The argument must have been returned by tzalloc().  */
 _GL_FUNCDECL_SYS (tzfree, void, (timezone_t __tz));
 _GL_CXXALIAS_SYS (tzfree, void, (timezone_t __tz));
+
+/* localtime_rz (tz, &t, &result)
+   Converts an absolute time T to a broken-down time RESULT, assuming the
+   time zone TZ.
+   This function is like 'localtime_r', but relies on the argument TZ instead
+   of an implicit global time zone.  */
 _GL_FUNCDECL_SYS (localtime_rz, struct tm *,
                   (timezone_t __tz, time_t const *restrict __timer,
                    struct tm *restrict __result) _GL_ARG_NONNULL ((2, 3)));
 _GL_CXXALIAS_SYS (localtime_rz, struct tm *,
                   (timezone_t __tz, time_t const *restrict __timer,
                    struct tm *restrict __result));
+
+/* mktime_z (tz, &tm)
+   Normalizes the broken-down time TM and converts it to an absolute time,
+   assuming the time zone TZ.  Returns the absolute time.
+   This function is like 'mktime', but relies on the argument TZ instead
+   of an implicit global time zone.  */
 _GL_FUNCDECL_SYS (mktime_z, time_t,
-                  (timezone_t __tz, struct tm *restrict __result)
+                  (timezone_t __tz, struct tm *restrict __tm)
                   _GL_ARG_NONNULL ((2)));
 _GL_CXXALIAS_SYS (mktime_z, time_t,
-                  (timezone_t __tz, struct tm *restrict __result));
+                  (timezone_t __tz, struct tm *restrict __tm));
+
+/* Time zone abbreviation strings (returned by 'localtime_rz' or 'mktime_z'
+   in the 'tm_zone' member of 'struct tm') are valid as long as
+     - the 'struct tm' argument is not destroyed or overwritten,
+   and
+     - the 'timezone_t' argument is not freed through tzfree().  */
+
 # endif
 
 /* Convert TM to a time_t value, assuming UTC.  */
diff --git a/modules/time_rz b/modules/time_rz
index 6a2b23c..ebcef84 100644
--- a/modules/time_rz
+++ b/modules/time_rz
@@ -1,17 +1,6 @@
 Description:
 Reentrant time zone functions: localtime_rz, mktime_z, etc.
 
-Comment:
-This implements the NetBSD-inspired extensions to <time.h>, which
-defines a type timezone_t and associated allocation functions tzalloc
-and tzfree, along with two functions localtime_rz and mktime_z that
-are like localtime_r and mktime except they have a new leading
-timezone_t argument.  Time zone abbreviation strings have lifetimes
-equal to the corresponding struct tm or timezone_t object (whichever
-is less).  tzalloc (X) yields a time zone object equivalent to setting
-the TZ environment variable to X.  tzalloc (NULL) is the same as an
-unset TZ environment variable.  (timezone_t) 0 stands for UTC.
-
 Files:
 lib/time-internal.h
 lib/time_rz.c
-- 
2.7.4

Reply via email to