Date: Fri, 27 Jul 2001 21:44:12 -0700 From: "David O'Brien" <obrien@freebsd.org> To: freebsd-hackers@freebsd.org Subject: [PATCH] reduce text(code) size and improve clarity of pkg_add Message-ID: <20010727214412.A91434@dragon.nuxi.com>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010727214412.A91434>