[bug-gnulib] Re: nit in strftime.c

[Jim Meyering]

Eric Blake <address@hidden> wrote:
> Paul Eggert <eggert <at> CS.UCLA.EDU> writes:
>> Unfortunately we cannot rely only on POSIX guarantees here, because
>> POSIX says that the contents of the output buffer are unspecified if
>> the output buffer is too small.  A failing POSIX strftime can set the
>> first byte of the output buffer to NUL.
>>
>> That being said, I like that patch since it's more likely that
>> strftime won't arbitrarily set ubuf[0]=='\0'.  To save you the time I
>> installed the patch into gnulib and coreutils.
>
> Why not prime the buffer?  Pass in an additional character in ufmt on the 
> front
> side (such as " %p" instead of "%p"), then ignore it on return, to distinguish
> between failure (return == 0) and success (return > 0, length of interest is
> return - 1).  Then you are not relying on (theoretical) non-portable behavior,
> and can distinguish between %p that expands to nothing vs. %p that overflows
> sizeof ubuf.

That is better.  Thanks!
How about this patch?

2005-03-15  Jim Meyering  <address@hidden>

        * strftime.c (my_strftime): Prepend a byte to the format string
        that's passed to the underlying strftime, solely to ensure that
        a return value of zero now reliably indicates failure.
        This also reverts yesterday's change.
        Suggestion from Eric Blake.

Index: lib/strftime.c
===================================================================
RCS file: /fetish/cu/lib/strftime.c,v
retrieving revision 1.81
diff -u -p -r1.81 strftime.c
--- lib/strftime.c      14 Mar 2005 23:23:25 -0000      1.81
+++ lib/strftime.c      15 Mar 2005 23:03:13 -0000
@@ -759,7 +759,7 @@ my_strftime (CHAR_T *s, size_t maxsize,
          {
            /* The relevant information is available only via the
               underlying strftime implementation, so use that.  */
-           char ufmt[4];
+           char ufmt[5];
            char *u = ufmt;
            char ubuf[1024]; /* enough for any single format in practice */
            size_t len;
@@ -771,16 +771,20 @@ my_strftime (CHAR_T *s, size_t maxsize,
            size_t strftime ();
 # endif
 
+           /* Prepending this byte ensures that the expansion will have
+              length of at least 1.  Otherwise, it'd be hard to distinguish
+              strftime failure (albeit unlikely) from what happens when %p
+              expands to the empty string.  */
+           *u++ = ' ';
            *u++ = '%';
            if (modifier != 0)
              *u++ = modifier;
            *u++ = format_char;
            *u = '\0';
-           ubuf[0] = '\1';
            len = strftime (ubuf, sizeof ubuf, ufmt, tp);
-           if (len == 0 && ubuf[0] != '\0')
+           if (len == 0)
              return 0;
-           cpy (len, ubuf);
+           cpy (len - 1, ubuf + 1);
          }
          break;
 #endif