From owner-freebsd-hackers Fri Jul 27 21:44:28 2001 Delivered-To: freebsd-hackers@freebsd.org Received: from dragon.nuxi.com (trang.nuxi.com [206.40.252.115]) by hub.freebsd.org (Postfix) with ESMTP id 45B8337B40A for ; Fri, 27 Jul 2001 21:44:16 -0700 (PDT) (envelope-from obrien@NUXI.com) Received: (from obrien@localhost) by dragon.nuxi.com (8.11.3/8.11.1) id f6S4iCo96881 for freebsd-hackers@freebsd.org; Fri, 27 Jul 2001 21:44:12 -0700 (PDT) (envelope-from obrien) Date: Fri, 27 Jul 2001 21:44:12 -0700 From: "David O'Brien" To: freebsd-hackers@freebsd.org Subject: [PATCH] reduce text(code) size and improve clarity of pkg_add Message-ID: <20010727214412.A91434@dragon.nuxi.com> Reply-To: obrien@freebsd.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i X-Operating-System: FreeBSD 5.0-CURRENT Organization: The NUXI BSD group X-Pgp-Rsa-Fingerprint: B7 4D 3E E9 11 39 5F A3 90 76 5D 69 58 D9 98 7A X-Pgp-Rsa-Keyid: 1024/34F9F9D5 Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG I'd like to apply this patch to pkg_add which reduces the amount of code the compiler generates, and improves the clarity of the code. 1. s_strl* is obvious some form of "safe" strl{cpy,cat}. But *WHAT* does it make "safe"? Isn't obvious w/o having to track down the s_strl{cat,cpy} function definitions. 2. The current code has more function call overhead than is needed. And it reduces the size of the basic block for the optimizer to work on. It also potentially has cache miss penalties. Index: add/main.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/main.c,v retrieving revision 1.42 retrieving revision 1.43 diff -u -r1.42 -r1.43 --- add/main.c 2001/07/26 20:25:50 1.42 +++ add/main.c 2001/07/28 01:59:58 1.43 @@ -111,7 +111,7 @@ break; case 't': - if (s_strlcpy(FirstPen, optarg, sizeof(FirstPen))) + if (strlcpy(FirstPen, optarg, sizeof(FirstPen)) > sizeof(FirstPen)) errx(1, "-t Argument too long."); break; @@ -145,27 +145,27 @@ if (Remote) { if ((packagesite = getpackagesite()) == NULL) errx(1, "package name too long"); - if (s_strlcpy(temppackageroot, packagesite, - sizeof(temppackageroot))) + if (strlcpy(temppackageroot, packagesite, + sizeof(temppackageroot)) >= sizeof(temppackageroot)) errx(1, "package name too long"); - if (s_strlcat(temppackageroot, *argv, - sizeof(temppackageroot))) + if (strlcat(temppackageroot, *argv, + sizeof(temppackageroot)) >= sizeof(temppackageroot)) errx(1, "package name too long"); remotepkg = temppackageroot; if (!((ptr = strrchr(remotepkg, '.')) && ptr[1] == 't' && ptr[2] == 'g' && ptr[3] == 'z' && !ptr[4])) - if (s_strlcat(remotepkg, ".tgz", sizeof(temppackageroot))) + if (strlcat(remotepkg, ".tgz", sizeof(temppackageroot)) >= sizeof(temppackageroot)) errx(1, "package name too long"); } if (!strcmp(*argv, "-")) /* stdin? */ pkgs[ch] = "-"; else if (isURL(*argv)) { /* preserve URLs */ - if (s_strlcpy(pkgnames[ch], *argv, sizeof(pkgnames[ch]))) + if (strlcpy(pkgnames[ch], *argv, sizeof(pkgnames[ch])) >= sizeof(pkgnames[ch])) errx(1, "package name too long"); pkgs[ch] = pkgnames[ch]; } else if ((Remote) && isURL(remotepkg)) { - if (s_strlcpy(pkgnames[ch], remotepkg, sizeof(pkgnames[ch]))) + if (strlcpy(pkgnames[ch], remotepkg, sizeof(pkgnames[ch])) >= sizeof(pkgnames[ch])) errx(1, "package name too long"); pkgs[ch] = pkgnames[ch]; } else { /* expand all pathnames to fullnames */ @@ -174,11 +174,11 @@ else { /* look for the file in the expected places */ if (!(cp = fileFindByPath(NULL, *argv))) { /* let pkg_do() fail later, so that error is reported */ - if (s_strlcpy(pkgnames[ch], *argv, sizeof(pkgnames[ch]))) + if (strlcpy(pkgnames[ch], *argv, sizeof(pkgnames[ch])) >= sizeof(pkgnames[ch])) errx(1, "package name too long"); pkgs[ch] = pkgnames[ch]; } else { - if (s_strlcpy(pkgnames[ch], cp, sizeof(pkgnames[ch]))) + if (strlcpy(pkgnames[ch], cp, sizeof(pkgnames[ch])) >= sizeof(pkgnames[ch])) errx(1, "package name too long"); pkgs[ch] = pkgnames[ch]; } @@ -220,37 +220,37 @@ struct utsname u; if (getenv("PACKAGESITE")) { - if (s_strlcpy(sitepath, getenv("PACKAGESITE"), - sizeof(sitepath))) + if (strlcpy(sitepath, getenv("PACKAGESITE"), + sizeof(sitepath)) >= sizeof(sitepath)) return NULL; return sitepath; } if (getenv("PACKAGEROOT")) { - if (s_strlcpy(sitepath, getenv("PACKAGEROOT"), sizeof(sitepath))) + if (strlcpy(sitepath, getenv("PACKAGEROOT"), sizeof(sitepath)) >= sizeof(sitepath)) return NULL; } else { - if (s_strlcat(sitepath, "ftp://ftp.freebsd.org", sizeof(sitepath))) + if (strlcat(sitepath, "ftp://ftp.freebsd.org", sizeof(sitepath)) >= sizeof(sitepath)) return NULL; } - if (s_strlcat(sitepath, "/pub/FreeBSD/ports/", sizeof(sitepath))) + if (strlcat(sitepath, "/pub/FreeBSD/ports/", sizeof(sitepath)) >= sizeof(sitepath)) return NULL; uname(&u); - if (s_strlcat(sitepath, u.machine, sizeof(sitepath))) + if (strlcat(sitepath, u.machine, sizeof(sitepath)) >= sizeof(sitepath)) return NULL; reldate = getosreldate(); for(i = 0; releases[i].directory != NULL; i++) { if (reldate >= releases[i].lowver && reldate <= releases[i].hiver) { - if (s_strlcat(sitepath, releases[i].directory, sizeof(sitepath))) + if (strlcat(sitepath, releases[i].directory, sizeof(sitepath)) >= sizeof(sitepath)) return NULL; break; } } - if (s_strlcat(sitepath, "/Latest/", sizeof(sitepath))) + if (strlcat(sitepath, "/Latest/", sizeof(sitepath)) >= sizeof(sitepath)) return NULL; return sitepath; Index: lib/str.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/pkg_install/lib/str.c,v retrieving revision 1.9 retrieving revision 1.10 diff -u -r1.9 -r1.10 --- lib/str.c 2001/05/12 09:44:32 1.9 +++ lib/str.c 2001/07/28 01:59:58 1.10 @@ -59,20 +59,6 @@ else *str = fileGetContents(s); return *str; -} - -/* Do a strlcpy and test for overflow */ -int -s_strlcpy(char *dst, const char *src, size_t size) -{ - return (strlcpy(dst, src, size) >= size); -} - -/* Do a strlcat and test for overflow */ -int -s_strlcat(char *dst, const char *src, size_t size) -{ - return (strlcat(dst, src, size) >= size); } /* Rather Obvious */ -- -- David (obrien@FreeBSD.org) To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message