Re: [bug] zsh/datetime strftime bug

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Sun, 9 Dec 2007 04:14:00 +0100
"Mikael Magnusson" <mikachu@xxxxxxxxx> wrote:
> > Hi zsh workers !
> >
> > I think I found a bug in strftime:
> >
> > baha@Saito% zsh -f
> > Saito%zmodload zsh/datetime
> > Saito% strftime '%T %x' $EPOCHSECONDS
> > 23:04:17

Yep, that's quite nasty since it's actually a basic problem with
the zsh interface layer to strftime that occurs in prompts, too.  It's
better hidden in the case of prompts because a longer multiplier of the
format string length is used to attempt to create the date/time string.

The basic problem is actually the interface to strftime:  it doesn't
tell you what size of buffer it needs nor allocate it's own memory.
(We've been here before...)  ztrftime() tries to help by returning -1 if
it thinks memory was too short for strftime()... but whoever added that
didn't bother changing the existing returns from returning 0 when it
knows definitely memory was too short.  So the caller blithely assumed 0
just meant the string was zero length, which is correct for the
strftime() interface but not for ztrftime.

I checked this fixed the basic problem first, but also increased the
multiplier of the format length in datetime to be the same as that for
prompts.

Various lessons include (i) the C library is antiquated (ii) if you're
trying to work around it, you need to do it properly (iii) we need yet
more tests, which I haven't done.

Note that writing tests
- doesn't require knowledge of the shell's internals
- is a very good way of learning the shell's syntax in some detail
- is a self-contained problem, not requiring any knowledge beyond
  how the test system works (which isn't complicated except for
  interactive tests)
- is highly beneficial to the quality of the finished product.

> I also noticed this just now,
> {4:11:47:~code/zsh}% grep parameter\ interface Src/Modules/*.c
> Src/Modules/datetime.c: * datetime.c - parameter interface to langinfo
> via curses
> Src/Modules/langinfo.c: * langinfo.c - parameter interface to langinfo
> via curses
> 
> I'm guessing it should say 'parameter interface to strftime' or something.

Indeed.

Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.173
diff -u -r1.173 utils.c
--- Src/utils.c	7 Dec 2007 11:34:50 -0000	1.173
+++ Src/utils.c	9 Dec 2007 14:39:24 -0000
@@ -2389,7 +2389,7 @@
 	     * Fix up some longer cases specially when we get to them.
 	     */
 	    if (ztrftimebuf(&bufsize, 2))
-		return 0;
+		return -1;
 	    switch (*fmt++) {
 	    case 'd':
 		*buf++ = '0' + tm->tm_mday / 10;
@@ -2447,12 +2447,12 @@
 #ifndef HAVE_STRFTIME
 	    case 'a':
 		if (ztrftimebuf(&bufsize, strlen(astr[tm->tm_wday]) -
2))
-		    return 0;
+		    return -1;
 		strucpy(&buf, astr[tm->tm_wday]);
 		break;
 	    case 'b':
 		if (ztrftimebuf(&bufsize, strlen(estr[tm->tm_mon]) -
2))
-		    return 0;
+		    return -1;
 		strucpy(&buf, estr[tm->tm_mon]);
 		break;
 	    case 'p':
@@ -2487,7 +2487,7 @@
 	    }
 	} else {
 	    if (ztrftimebuf(&bufsize, 1))
-		return 0;
+		return -1;
 	    *buf++ = *fmt++;
 	}
     *buf = '\0';
Index: Src/Modules/datetime.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/datetime.c,v
retrieving revision 1.17
diff -u -r1.17 datetime.c
--- Src/Modules/datetime.c	6 Jul 2007 21:52:40 -0000	1.17
+++ Src/Modules/datetime.c	9 Dec 2007 14:39:24 -0000
@@ -1,5 +1,5 @@
 /*
- * datetime.c - parameter interface to langinfo via curses
+ * datetime.c - parameter and command interface to date and time
utilities *
  * This file is part of zsh, the Z shell.
  *
@@ -121,7 +121,7 @@
     }
 
     t = localtime(&secs);
-    bufsize = strlen(argv[0]) * 2;
+    bufsize = strlen(argv[0]) * 8;
     buffer = zalloc(bufsize);
 
     for (x=0; x < 4; x++) {

-- 
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/