From owner-cvs-all Tue Aug 7 15:17:25 2001 Delivered-To: cvs-all@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id AE83237B401; Tue, 7 Aug 2001 15:17:08 -0700 (PDT) (envelope-from des@ofug.org) Received: (from des@localhost) by flood.ping.uio.no (8.9.3/8.9.3) id AAA08194; Wed, 8 Aug 2001 00:17:07 +0200 (CEST) (envelope-from des@ofug.org) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Kris Kennaway Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/lib/libc/string strlcat.c References: <200107241134.f6OBYMr33205@freefall.freebsd.org> From: Dag-Erling Smorgrav Date: 08 Aug 2001 00:17:05 +0200 In-Reply-To: <200107241134.f6OBYMr33205@freefall.freebsd.org> Message-ID: Lines: 49 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Kris Kennaway 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