nstrftime %c bug w.r.t. time zone

nstrftime %c bug w.r.t. time zone

[Bruno Haible]

Hi Paul,

time.in.h says:

  (timezone_t) NULL stands for UTC.

However, nstrftime, on NetBSD 7 and Solaris 11, prints my local time zone
("CET"), not "UTC" or "GMT", in this test program:

================================== foo.c =================================
#include <config.h>
#include <assert.h>
#include <locale.h>
#include <stdio.h>
#include <string.h>
#include <time.h>
#include "strftime.h"
int main ()
{
  setlocale (LC_ALL, "fr_FR.UTF-8");

  time_t t = 1509000003;
  struct tm *tm = gmtime (&t);
  int ns = 123456789;
  char buf[100];
  size_t n;

  n = nstrftime (buf, sizeof buf, "%+4Y-%m-%d %H:%M:%S.%N", tm, 0, ns);
  assert (n > 0);
  printf ("Format as ISO 8601: %s\n", buf);
  assert (strcmp (buf, "2017-10-26 06:40:03.123456789") == 0);

  n = nstrftime (buf, sizeof buf, "%c", tm, 0, ns);
  assert (n > 0);
  printf ("%%c directive: %s\n", buf);
}
/* Output on
NetBSD 7.1:   26 octobre 2017 06:40:03 CET
Solaris 11.4: 26 octobre 2017 à 06:40:03 CET
*/
============================================================================
How to run it:
- Use a testdir that contains module 'nstrftime'.
- Build it.
- $ gcc -Wall foo.c -I. -Igllib -I../gllib gllib/libgnu.a
  $ ./a.out

Another way to reproduce it:
  1. Create a testdir for module 'nstrftime'.
  2. Transport it to a NetBSD or Solaris system.
  3. Run it. Result: test-nstrftime-2.sh fails at test-nstrftime.h:421.

Bruno

Re: nstrftime %c bug w.r.t. time zone

[Bruno Haible]

I wrote:

> time.in.h says:
> 
>   (timezone_t) NULL stands for UTC.
> 
> However, nstrftime, on NetBSD 7 and Solaris 11, prints my local time zone
> ("CET"), not "UTC" or "GMT", in this test program:

This patch provides a workaround, and thus removes the test-nstrftime-2.sh
failure at test-nstrftime.h:421.


2024-02-08  Bruno Haible  <bruno@clisp.org>

	nstrtime, c-nstrftime: Fix %c directive's result on NetBSD, Solaris.
	* lib/strftime.c (__strftime_internal): On NetBSD and Solaris, remove
	the last word of the %c directive's result if it looks like a time zone.

diff --git a/lib/strftime.c b/lib/strftime.c
index f6714524bd..b70494cf78 100644
--- a/lib/strftime.c
+++ b/lib/strftime.c
@@ -967,7 +967,48 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize)
             len = strftime (ubuf, sizeof ubuf, ufmt, tp);
 # endif
             if (len != 0)
-              cpy (len - 1, ubuf + 1);
+              {
+# if defined __NetBSD__ || defined __sun /* NetBSD, Solaris */
+                if (format_char == L_('c'))
+                  {
+                    /* The output of the strftime %c directive consists of the
+                       date, the time, and the time zone.  But the time zone is
+                       wrong, since neither TZ nor ZONE was passed as argument.
+                       Therefore, remove the the last space-delimited word.
+                       In order not to accidentally remove a date or a year
+                       (that contains no letter) or an AM/PM indicator (that has
+                       length 2), remove that last word only if it contains a
+                       letter and has length >= 3.  */
+                    char *space;
+                    for (space = ubuf + len - 1; *space != ' '; space--)
+                      ;
+                    if (space > ubuf)
+                      {
+                        /* Found a space.  */
+                        if (strlen (space + 1) >= 3)
+                          {
+                            /* The last word has length >= 3.  */
+                            bool found_letter = false;
+                            const char *p;
+                            for (p = space + 1; *p != '\0'; p++)
+                              if ((*p >= 'A' && *p <= 'Z')
+                                  || (*p >= 'a' && *p <= 'z'))
+                                {
+                                  found_letter = true;
+                                  break;
+                                }
+                            if (found_letter)
+                              {
+                                /* The last word contains a letter.  */
+                                *space = '\0';
+                                len = space - ubuf;
+                              }
+                          }
+                      }
+                  }
+# endif
+                cpy (len - 1, ubuf + 1);
+              }
           }
           break;
 #endif

Re: nstrftime %c bug w.r.t. time zone

[Bruno Haible]

> 2024-02-08  Bruno Haible  <bruno@clisp.org>
> 
> 	nstrtime, c-nstrftime: Fix %c directive's result on NetBSD, Solaris.
> 	* lib/strftime.c (__strftime_internal): On NetBSD and Solaris, remove
> 	the last word of the %c directive's result if it looks like a time zone.

This patch adds a mention of the problem to the documentation:


diff --git a/doc/posix-functions/strftime.texi b/doc/posix-functions/strftime.texi
index 7c4c0391fa..b62ea37fdb 100644
--- a/doc/posix-functions/strftime.texi
+++ b/doc/posix-functions/strftime.texi
@@ -16,6 +16,11 @@
 Portability problems not fixed by Gnulib:
 @itemize
 @item
+The %c specifier does not work with a time retrieved through @code{gmtime}
+or @code{gmtime_r}, since it outputs also the current time zone,
+on some platforms:
+NetBSD 9.3, Solaris 11.4.
+@item
 The %r specifier produces empty output, at least in a French locale,
 on some platforms:
 macOS 12.5, FreeBSD 14.0.

Re: nstrftime %c bug w.r.t. time zone

[Paul Eggert]

On 2024-02-08 03:50, Bruno Haible wrote:
> This patch provides a workaround, and thus removes the test-nstrftime-2.sh
> failure at test-nstrftime.h:421.

I was thinking of something a bit more general: temporarily set the time 
zone before calling the underlying strftime, and reset it afterwards. 
This of course won't work in a multithreaded system without some 
locking, but we can either add the locking or not bother.

time_rz.c already does this sort of thing, and I suppose it could export 
its secrets to nstrftime.c.

Re: nstrftime %c bug w.r.t. time zone

[Bruno Haible]

Paul Eggert wrote:
> > This patch provides a workaround, and thus removes the test-nstrftime-2.sh
> > failure at test-nstrftime.h:421.
> 
> I was thinking of something a bit more general: temporarily set the time 
> zone before calling the underlying strftime, and reset it afterwards.
> This of course won't work in a multithreaded system ...

Yes, this is not MT-safe. For 20 years I've been training myself to avoid
thinking in a non-MT-safe way, and apparently now in this case it worked :-)

> time_rz.c already does this sort of thing, and I suppose it could export 
> its secrets to nstrftime.c.

Ouch ouch. No, instead of making more code *not* MT-safe, we should make more
code MT-safe!

Indeed, time_rz.c with its setenv_TZ function is not MT-safe, and with it
also the modules
  nstrftime
  c-nstrftime
  fprintftime
  parse-datetime
!!

This is a show-stopper, since we've been discussing nstrftime and c-nstrftime,
as replacements to recommend to developers, for three days. And of course we
cannot recommend functions that look MT-safe but are not.

Bruno