From owner-cvs-usrbin Fri Mar 7 08:55:48 1997 Return-Path: Received: (from root@localhost) by freefall.freebsd.org (8.8.5/8.8.5) id IAA27580 for cvs-usrbin-outgoing; Fri, 7 Mar 1997 08:55:48 -0800 (PST) Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.19]) by freefall.freebsd.org (8.8.5/8.8.5) with ESMTP id IAA27572; Fri, 7 Mar 1997 08:55:40 -0800 (PST) Received: (from bde@localhost) by godzilla.zeta.org.au (8.8.3/8.6.9) id DAA22618; Sat, 8 Mar 1997 03:53:13 +1100 Date: Sat, 8 Mar 1997 03:53:13 +1100 From: Bruce Evans Message-Id: <199703071653.DAA22618@godzilla.zeta.org.au> To: bde@zeta.org.au, yokota@zodiac.mech.utsunomiya-u.ac.jp Subject: Re: cvs commit: src/usr.bin/w pr_time.c w.c Cc: cvs-all@freefall.freebsd.org, CVS-committers@freefall.freebsd.org, cvs-usrbin@freefall.freebsd.org, yokota@freefall.freebsd.org Sender: owner-cvs-usrbin@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk >>Strings are null-terminated by definition. According to the ANSI C >>standard, strftime() gives either a string (if there is enough space) >>or indeterminate buffer contents (otherwise) according to the ANSI C >>standard... >Maybe I should have said "Put 0 at the end of the buffer used by >strftime() so that the buffer is null-terminated even when strftime() >doesn't place 0 there." This depends on the buffer having something useful in it. If we're going to depend on unportable behaviour, then we might as well change the library to force null termination (except in the silly case where the buffer size is 0). That's probably what strftime() should have been specified to do. >What I did was something like this in a couple of places. > >--- pr_time.c-dist Tue Feb 11 19:49:24 1997 >+++ pr_time.c Sun Feb 23 11:55:20 1997 >@@ -76,7 +76,8 @@ > (void)strcpy(fmt, __CONCAT("%l:%", "M%p")); > } > >- (void)strftime(buf, sizeof(buf), fmt, tp); >+ (void)strftime(buf, sizeof(buf) - 1, fmt, tp); >+ buf[sizeof(buf) - 1] = '\0'; > (void)printf("%s", buf); > } I think it should do something like this: if (strftime(...) == 0) (void)strftime(buf, sizeof(buf), some_fixed_width_format, tp); The format is "%d%b%y", "%a%I%p" or "%l%M%p". According to the FreeBSD docs, these have widths 2+3+2, 3+2+unspecified and 2+2+unspecified, respectively. The ISO C docs don't seem to specify the widths of %b or %A, so the widths of all the formats are unspecified :-(. Otherwise "%d%b%y" would be a good fixed size format. I would try %d\?\?\?%y". The buffer size is 256 so the first strftime() will never fail in practice :-). However, we really want a width of 7, so using a buffer size of 8 would be good for finding errors early. >As far as I can understand from src/lib/libc/stdtime/strftime.c, the >FreeBSD version of strftime() does not truncate the string when there >is not enough space in the buffer, rather it stuffs as many characters >into the buffer as possible and returns without a terminal zero. Thus, >it follows the ANSI C standard, does it not? It does more than the standard requires. >I am bit confused. Should I back out the patch? Well, it wasn't urgent. It wasn't even necessary in the CSRG version since only the C locale was supported so the string width was guaranteed to be 7. The programmer must have been lazy to use a buffer size of 256 :-). Now the main bug is that the string width is not guaranteed to be 7. Worse, the width may vary with the login time, so the columns may be messed up. Bruce