From owner-freebsd-standards Fri Nov 30 2:41:22 2001 Delivered-To: freebsd-standards@freebsd.org Received: from khavrinen.lcs.mit.edu (khavrinen.lcs.mit.edu [18.24.4.193]) by hub.freebsd.org (Postfix) with ESMTP id 3711737B416 for ; Fri, 30 Nov 2001 02:41:15 -0800 (PST) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by khavrinen.lcs.mit.edu (8.11.4/8.11.4) with ESMTP id fAUAfCG24153 for ; Fri, 30 Nov 2001 05:41:13 -0500 (EST) (envelope-from bde@zeta.org.au) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id VAA21782; Fri, 30 Nov 2001 21:40:57 +1100 Date: Fri, 30 Nov 2001 21:41:04 +1100 (EST) From: Bruce Evans X-X-Sender: To: Wes Peters Cc: Bill Fenner , , Subject: Re: strerror_r() implementation In-Reply-To: <3C05BE3A.1DF1A539@softweyr.com> Message-ID: <20011130210640.X752-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-standards@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Wed, 28 Nov 2001, Wes Peters wrote: > Ah, I see the confusion. The size of ebuf in strerror doesn't need to > be big enough to hold messages from sys_errlist, it only needs to be > large enough to hold "Unknown error: " plus the length of a 64-bit > number in ascii. > > How does this look? > > --- strerror.c.nxt Wed Nov 28 20:32:54 2001 > +++ strerror.c Wed Nov 28 20:39:30 2001 > @@ -86,18 +86,17 @@ > } > > > -/* > - * NOTE: the following length should be enough to hold the longest defined > - * error message in sys_errlist, defined in ../gen/errlst.c. This is a WAG > - * that is better than the previous value. > - */ > -#define ERR_LEN 64 > - > char * > strerror(num) > int num; > { > unsigned int uerr; > + > + /* > + * NOTE: the following length should be enough to hold the > + * longest "Unknown error: " message above. > + */ > +#define ERR_LEN 55 > static char ebuf[ERR_LEN]; > > uerr = num; /* convert to unsigned */ > @@ -106,7 +105,7 @@ > > /* strerror can't fail so handle truncation semi-elegantly */ > if (strerror_r(num, ebuf, (size_t) ERR_LEN) != 0) > - ebuf[ERR_LEN - 1] = '\0'; > + ebuf[ERR_LEN - 1] = '\0'; > > return ebuf; > } I prefer the version enclosed at the end. It has the following additional non-formatting changes: - parametrize the buffer size according to the sizes in strerror_r() - unobfuscate uerr (don't use an unsigned variable just to hand-optimize for 20-year-old compilers). - remove redundant code (the strerror_r() can't fail unless we used the wrong buffer size, and we should get this right by not hard-coding the size of tmp[]) `tmp' should have a better name now that it is more global. I didn't change it because that would require large changes in strerror_r(). > The last change is a whitespace error. Mustn't have any style(9) nits, > right? ;^) I agree, so about 10 more of them in strerror() alone ;^). They were in the following classes: (1) missing parens around return values (2) comments not English sentences (3) excessive vertical whitespace (KNF uses indent -sob) (4) excessive indentation for comments at the right of code (32 is normal). The comment fixes might not be obvious because the comments just went away. All of these bugs except (4) were not in rev.1.4. %%% Index: strerror.c =================================================================== RCS file: /home/ncvs/src/lib/libc/string/strerror.c,v retrieving revision 1.5 diff -u -2 -r1.5 strerror.c --- strerror.c 27 Nov 2001 07:39:46 -0000 1.5 +++ strerror.c 30 Nov 2001 10:08:58 -0000 @@ -42,12 +42,13 @@ #include +#define UPREFIX "Unknown error: " + +static char tmp[40]; /* 64-bit number + slop */ int strerror_r(int errnum, char *strerrbuf, size_t buflen) { -#define UPREFIX "Unknown error: " unsigned int uerr; char *p, *t; - char tmp[40]; /* 64-bit number + slop */ int len; @@ -84,30 +85,15 @@ } - -/* - * NOTE: the following length should be enough to hold the longest defined - * error message in sys_errlist, defined in ../gen/errlst.c. This is a WAG - * that is better than the previous value. - */ -#define ERR_LEN 64 - char * strerror(num) int num; { - unsigned int uerr; - static char ebuf[ERR_LEN]; + static char ebuf[sizeof(UPREFIX) - 1 + sizeof(tmp)]; - uerr = num; /* convert to unsigned */ - if (uerr < sys_nerr) - return (char *)sys_errlist[uerr]; - - /* strerror can't fail so handle truncation semi-elegantly */ - if (strerror_r(num, ebuf, (size_t) ERR_LEN) != 0) - ebuf[ERR_LEN - 1] = '\0'; - - return ebuf; + if (num >= 0 && num < sys_nerr) + return ((char *)sys_errlist[num]); + (void)strerror_r(num, ebuf, sizeof(ebuf)); + return (ebuf); } - #ifdef STANDALONE_TEST %%% This has not been tested. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-standards" in the body of the message