[PATCH] Use Gnulib's `strftime'

[PATCH] Use Gnulib's `strftime'

[Ludovic Courtès]

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

Hello!

I'm planning to use Gnulib's `strftime' module on `master' to fix
portability problems related to `strftime', aka. #24130
(https://savannah.gnu.org/bugs/?24130).  The good thing is that
`strftime' will now work the same regardless of the underlying libc.

The source modification is attached.  For a discussion of
`nstrftime ()', see
http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/14380 .

Please let me know if you think of a good reason to not do this,
otherwise I'll commit it within a week or so.

Thanks,
Ludo'.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: The patch. --]
[-- Type: text/x-patch, Size: 6389 bytes --]

From 69f23174d313650ca8fb0f69ede45c48d7a26b05 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Tue, 2 Sep 2008 21:24:53 +0200
Subject: [PATCH] Use Gnulib's `strftime' to address bug #24130.

* libguile/stime.c (scm_strftime): Use `nstrftime ()' from Gnulib.
  This provides the same semantics on all platforms, thereby fixing
  bug #24130.

* doc/ref/posix.texi (Time): Remove note about non-portable `%Z'
  behavior.  Describe the new, portable behavior.

* test-suite/tests/time.test ("strftime")["strftime %Z doesn't return
  garbage"]: Reinstate.
  ["C99 %z format"](have-strftime-%z): Remove.
  ("GMT", "EST+5"): Don't use `have-strftime-%z'.
---
 doc/ref/posix.texi         |   25 +++----------------------
 libguile/stime.c           |    8 ++++----
 test-suite/tests/time.test |   40 +++++++---------------------------------
 3 files changed, 14 insertions(+), 59 deletions(-)

diff --git a/doc/ref/posix.texi b/doc/ref/posix.texi
index 34194fb..a91bdb9 100644
--- a/doc/ref/posix.texi
+++ b/doc/ref/posix.texi
@@ -1,6 +1,6 @@
 @c -*-texinfo-*-
 @c This is part of the GNU Guile Reference Manual.
-@c Copyright (C)  1996, 1997, 2000, 2001, 2002, 2003, 2004, 2006, 2007
+@c Copyright (C)  1996, 1997, 2000, 2001, 2002, 2003, 2004, 2006, 2007, 2008
 @c   Free Software Foundation, Inc.
 @c See the file guile.texi for copying conditions.
 
@@ -1264,27 +1264,8 @@ formatting.
 If @code{setlocale} has been called (@pxref{Locales}), month and day
 names are from the current locale and in the locale character set.
 
-Note that @samp{%Z} might print the @code{tm:zone} in @var{tm} or it
-might print just the current zone (@code{tzset} above).  A GNU system
-prints @code{tm:zone}, a strict C99 system like NetBSD prints the
-current zone.  Perhaps in the future Guile will try to get
-@code{tm:zone} used always.
-@c
-@c  The issue in the above is not just whether tm_zone exists in
-@c  struct tm, but whether libc feels it should read it.  Being a
-@c  non-C99 field, a strict C99 program won't know to set it, quite
-@c  likely leaving garbage there.  NetBSD, which has the field,
-@c  therefore takes the view that it mustn't read it.  See the PR
-@c  about this at
-@c
-@c      http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=21722
-@c
-@c  Uniformly making tm:zone used on all systems (all those which have
-@c  %Z at all of course) might be nice (either mung TZ and tzset, or
-@c  mung tzname[]).  On the other hand it would make us do more than
-@c  C99 says, and we really don't want to get intimate with the gory
-@c  details of libc time funcs, no more than can be helped.
-@c
+Note that @samp{%Z} always ignores the @code{tm:zone} in @var{tm};
+instead it prints just the current zone (@code{tzset} above).
 @end deffn
 
 @deffn {Scheme Procedure} strptime format string
diff --git a/libguile/stime.c b/libguile/stime.c
index fa8b585..be5bf65 100644
--- a/libguile/stime.c
+++ b/libguile/stime.c
@@ -44,6 +44,7 @@
 
 #include <stdio.h>
 #include <errno.h>
+#include <strftime.h>
 
 #include "libguile/_scm.h"
 #include "libguile/async.h"
@@ -689,10 +690,9 @@ SCM_DEFINE (scm_strftime, "strftime", 2, 0, 0,
     tzset ();
 #endif
 
-    /* POSIX says strftime returns 0 on buffer overrun, but old
-       systems (i.e. libc 4 on GNU/Linux) might return `size' in that
-       case. */
-    while ((len = strftime (tbuf, size, myfmt, &t)) == 0 || len == size)
+    /* Use `nstrftime ()' from Gnulib, which supports all GNU extensions
+       supported by glibc.  */
+    while ((len = nstrftime (tbuf, size, myfmt, &t, 0, 0)) == 0)
       {
 	free (tbuf);
 	size *= 2;
diff --git a/test-suite/tests/time.test b/test-suite/tests/time.test
index ebc4499..d5639eb 100644
--- a/test-suite/tests/time.test
+++ b/test-suite/tests/time.test
@@ -1,7 +1,7 @@
 ;;;; time.test --- test suite for Guile's time functions     -*- scheme -*-
 ;;;; Jim Blandy <jimb@red-bean.com> --- June 1999, 2004
 ;;;;
-;;;; 	Copyright (C) 1999, 2004, 2006, 2007 Free Software Foundation, Inc.
+;;;; 	Copyright (C) 1999, 2004, 2006, 2007, 2008 Free Software Foundation, Inc.
 ;;;; 
 ;;;; This program is free software; you can redistribute it and/or modify
 ;;;; it under the terms of the GNU General Public License as published by
@@ -196,44 +196,19 @@
 
 (with-test-prefix "strftime"
 
-  ;; Note we must force isdst to get the ZOW zone name out of %Z on HP-UX.
-  ;; If localtime is in daylight savings then it will decide there's no
-  ;; daylight savings zone name for the fake ZOW, and come back empty.
-  ;;
-  ;; This test is disabled because on NetBSD %Z doesn't look at the tm_zone
-  ;; field in struct tm passed by guile.  That behaviour is reasonable
-  ;; enough since that field is not in C99 so a C99 program won't know it
-  ;; has to be set.  For the details on that see
-  ;;
-  ;;     http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=21722
-  ;;
-  ;; Not sure what to do about this in guile, it'd be nice for %Z to look at
-  ;; tm:zone everywhere.
-  ;;
-  ;;
-  ;; (pass-if "strftime %Z doesn't return garbage"
-  ;; 	 (let ((t (localtime (current-time))))
-  ;; 	   (set-tm:zone t "ZOW")
-  ;; 	   (set-tm:isdst t 0)
-  ;; 	   (string=? (strftime "%Z" t)
-  ;; 		     "ZOW")))
+  (pass-if "strftime %Z doesn't return garbage"
+    (let ((t (localtime (current-time))))
+      (set-tm:zone t "ZOW")
+      (set-tm:isdst t 0)
+      (string=? (strftime "%Z" t)
+                "ZOW")))
 
   (with-test-prefix "C99 %z format"
 
-    ;; C99 spec is empty string if no zone determinable
-    ;;
-    ;; on pre-C99 systems not sure what to expect if %z unsupported, probably
-    ;; "%z" unchanged in C99 if timezone
-    ;;
-    (define have-strftime-%z
-      (not (member (strftime "%z" (gmtime 0))
-		   '("" "%z"))))
-
     ;; %z here is quite possibly affected by the same tm:gmtoff vs current
     ;; zone as %Z above is, so in the following tests we make them the same.
 
     (pass-if "GMT"
-      (or have-strftime-%z (throw 'unsupported))
       (putenv "TZ=GMT+0")
       (tzset)
       (let ((tm (localtime 86400)))
@@ -243,7 +218,6 @@
     ;; because we didn't adjust for tm:gmtoff being west of Greenwich versus
     ;; tm_gmtoff being east of Greenwich
     (pass-if "EST+5"
-      (or have-strftime-%z (throw 'unsupported))
       (putenv "TZ=EST+5")
       (tzset)
       (let ((tm (localtime 86400)))
-- 
1.6.0

Re: [PATCH] Use Gnulib's `strftime'

[Ludovic Courtès]

Hi,

ludo@gnu.org (Ludovic Courtès) writes:

> I'm planning to use Gnulib's `strftime' module on `master' to fix
> portability problems related to `strftime', aka. #24130
> (https://savannah.gnu.org/bugs/?24130).  The good thing is that
> `strftime' will now work the same regardless of the underlying libc.
>
> The source modification is attached.  For a discussion of
> `nstrftime ()', see
> http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/14380 .
>
> Please let me know if you think of a good reason to not do this,
> otherwise I'll commit it within a week or so.

Applied.

Thanks,
Ludo'.

Re: [PATCH] Use Gnulib's `strftime'

[Neil Jerram]

Hi Ludo...

2008/9/2 Ludovic Courtès <ludo@gnu.org>:
> Hello!
>
> I'm planning to use Gnulib's `strftime' module on `master' to fix
> portability problems related to `strftime', aka. #24130
> (https://savannah.gnu.org/bugs/?24130).  The good thing is that
> `strftime' will now work the same regardless of the underlying libc.

Excellent!

> The source modification is attached.

I'm just a bit confused about %Z, because it seems to me that the
documentation contradicts the test.

Here's the doc change:

> +Note that @samp{%Z} always ignores the @code{tm:zone} in @var{tm};
> +instead it prints just the current zone (@code{tzset} above).

Here's the test:

> +  (pass-if "strftime %Z doesn't return garbage"
> +    (let ((t (localtime (current-time))))
> +      (set-tm:zone t "ZOW")
> +      (set-tm:isdst t 0)
> +      (string=? (strftime "%Z" t)
> +                "ZOW")))

The doc seems to be saying that (strftime "%Z" tm) will ignore TM's
zone, but the test makes it look like (strftime "%Z" tm) uses TM's
zone.  Am I misunderstanding?

Regards,
      Neil

Re: [PATCH] Use Gnulib's `strftime'

[Ludovic Courtès]

Hi Neil,

"Neil Jerram" <neiljerram@googlemail.com> writes:

> I'm just a bit confused about %Z, because it seems to me that the
> documentation contradicts the test.
>
> Here's the doc change:
>
>> +Note that @samp{%Z} always ignores the @code{tm:zone} in @var{tm};
>> +instead it prints just the current zone (@code{tzset} above).
>
> Here's the test:
>
>> +  (pass-if "strftime %Z doesn't return garbage"
>> +    (let ((t (localtime (current-time))))
>> +      (set-tm:zone t "ZOW")
>> +      (set-tm:isdst t 0)
>> +      (string=? (strftime "%Z" t)
>> +                "ZOW")))
>
> The doc seems to be saying that (strftime "%Z" tm) will ignore TM's
> zone, but the test makes it look like (strftime "%Z" tm) uses TM's
> zone.  Am I misunderstanding?

No, you're right.

That was a double mistake of my side: I misunderstood the UTC argument
to `nstrftime ()' as a boolean indicating whether to honor `tm_zone',
and overlooked the part of `scm_strftime ()' intended to "simulate" the
`tm_zone' fields on platforms that lack it...

Thanks!

Ludo'.