From owner-freebsd-audit Mon Jul 15 12:31: 7 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 52C6B37B405 for ; Mon, 15 Jul 2002 12:30:53 -0700 (PDT) Received: from mailsrv.otenet.gr (mailsrv.otenet.gr [195.170.0.5]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0605943E4A for ; Mon, 15 Jul 2002 12:30:52 -0700 (PDT) (envelope-from keramida@FreeBSD.org) Received: from hades.hell.gr (patr530-b146.otenet.gr [212.205.244.154]) by mailsrv.otenet.gr (8.12.4/8.12.4) with ESMTP id g6FJUnsC029362; Mon, 15 Jul 2002 22:30:49 +0300 (EEST) Received: from hades.hell.gr (hades [127.0.0.1]) by hades.hell.gr (8.12.5/8.12.5) with ESMTP id g6FJUmft058686; Mon, 15 Jul 2002 22:30:48 +0300 (EEST) (envelope-from keramida@FreeBSD.org) Received: (from charon@localhost) by hades.hell.gr (8.12.5/8.12.5/Submit) id g6FJUmNP058685; Mon, 15 Jul 2002 22:30:48 +0300 (EEST) (envelope-from keramida@FreeBSD.org) Date: Mon, 15 Jul 2002 22:30:48 +0300 From: Giorgos Keramidas To: Bruce Evans Cc: Dag-Erling Smorgrav , freebsd-audit@FreeBSD.org Subject: Re: bin/ln & WARNS=5 Message-ID: <20020715193047.GE55859@hades.hell.gr> References: <20020715111436.GD50130@hades.hell.gr> <20020716044254.G41571-100000@gamplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020716044254.G41571-100000@gamplex.bde.org> X-Operating-System: FreeBSD 5.0-CURRENT i386 X-PGP-Fingerprint: C1EB 0653 DB8B A557 3829 00F9 D60F 941A 3186 03B6 X-Phone: +30-944-116520 Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 2002-07-16 05:14 +0000, Bruce Evans wrote: > On Mon, 15 Jul 2002, Giorgos Keramidas wrote: > > const char *p; > > int ch, exists, first; > > char path[PATH_MAX]; > > + int pathlen; > > This unsorts the variables. So pathlen should be with the rest of those `int's. Right. > This takes more code than I like, but I guess something like it is required > to be strictly correct. I prefer: > > pathlen = snprintf(path, sizeof(path), "%s/%s", source, p); > if (pathlen < 0 || pathlen >= (int)sizeof(path))) { Warren has suggested that we check first if the buffer space would be enough and then use snprintf() [as I understood from a later post]. I'm equally happy with both versions, and I would really like to cover all the possible snprintf() return values; both positive and negative. It's not very clear if snprintf() can return a negative value, and it is definitely possible as some have pointed out in C99. I prefer guarding against this case too, just in case. So, we have now two versions of the diff to choose from. One that checks for the appropriate buffer size before snprintf() is called. I've changed sizeof to use () here, and sprintf/snprintf again. %%% Index: ln.c =================================================================== RCS file: /home/ncvs/src/bin/ln/ln.c,v retrieving revision 1.28 diff -u -r1.28 ln.c --- ln.c 30 Jun 2002 05:13:54 -0000 1.28 +++ ln.c 15 Jul 2002 18:20:19 -0000 @@ -189,12 +189,12 @@ p = target; else ++p; - if (snprintf(path, sizeof(path), "%s/%s", source, p) >= - sizeof(path)) { + if (strlen(source) + strlen(p) + 2 > sizeof(path)) { errno = ENAMETOOLONG; warn("%s", target); return (1); } + snprintf(path, sizeof(path), "%s/%s", source, p); source = path; } %%% And the second version is something that calls snprintf() anyway and then checks the result. %%% Index: ln.c =================================================================== RCS file: /home/ncvs/src/bin/ln/ln.c,v retrieving revision 1.28 diff -u -r1.28 ln.c --- ln.c 30 Jun 2002 05:13:54 -0000 1.28 +++ ln.c 15 Jul 2002 19:28:52 -0000 @@ -161,7 +161,7 @@ { struct stat sb; const char *p; - int ch, exists, first; + int ch, exists, first, pathlen; char path[PATH_MAX]; if (!sflag) { @@ -189,8 +189,8 @@ p = target; else ++p; - if (snprintf(path, sizeof(path), "%s/%s", source, p) >= - sizeof(path)) { + pathlen = snprintf(path, sizeof path, "%s/%s", source, p); + if (pathlen < 0 || pathlen >= (int)sizeof(path)) { errno = ENAMETOOLONG; warn("%s", target); return (1); %%% Both versions compile cleanly with WARNS=5. Which one do we feel better like using? - Giorgos To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message