Date: 08 Aug 2001 00:17:05 +0200 From: Dag-Erling Smorgrav <des@ofug.org> To: Kris Kennaway <kris@FreeBSD.org> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/lib/libc/string strlcat.c Message-ID: <xzpelqnecri.fsf@flood.ping.uio.no> In-Reply-To: <200107241134.f6OBYMr33205@freefall.freebsd.org> References: <200107241134.f6OBYMr33205@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Kris Kennaway <kris@FreeBSD.org> writes:
> Log:
> Sync to OpenBSD (update comment and minor style change).
Umm, this commit prompted me to have a look at the code, and it
looks a bit stupid.
First of all, the first while loop should be rewritten as:
while (n-- >= 0 && *d != '\0')
d++;
as the current version will blow up badly if the siz argument is
negative to start with (some may think that's a feature - but it's
more likely than not to blow the stack, so even if you do get a core
dump, and not, say, a root compromise, it's likely to be totally
undecipherable).
Second, the two lines that follow the loop are idiotic. They amount
to the single statement "n++", and can be totally eliminated if the
loop is rewritten to:
while (n >= 0 && *d != '\0')
d++, n--;
Third, you probably don't even *want* to do that, because having n one
less than the real "free amount" (and fixing "if (n == 0)" to "if (n <
0)") allows you to replace the "if (n != 1)" in the second while loop
with "if (n > 0)", which is more likely to compile to a single
instruction. So the two lines after the first while loop can go.
Fourth (this is optional) you probably want to split the second while
loop into a copying loop and a "mop-up" counting loop:
/* replace n-- with --n if you didn't follow suggestion #3 */
while (n-- > 0 && *s != '\0')
*d++ = *s++;
*d = '\0';
while (*s != '\0')
s++;
Fifth, the return statements do not conform to style(9).
Sixth, neither do the variable initializations. They can be
eliminated if the while loops are rewritten as for loops.
DES
--
Dag-Erling Smorgrav - des@ofug.org
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?xzpelqnecri.fsf>