Remove s_strl*(). I am not sure what was thought they accomplished.

When reading the code I had to stop, say "ok, what does *these*
modifications of strl*() do?  Pull out grep.  Oh, not in add/, maybe above
in ../lib/?  Yep.  So what do they do?  Comments above them are misleading,
guess I'll have to read the code.  Oh, they just test strl* against the
size and return the result of the test.  Now I can continue to read the
code I was.

The uses of s_strl*() then test that result and errx()'s.
Lets think about the "optimized" code I am removing:

In general the compiler pushes the three args to strl* onto the stack and calls
s_strl*.  s_strl* has to indirectly access 3 args from the stack.  Then push
them on the stack a 2nd time for the real strl* call.  s_strl* then pops the
return from strl* off the stack; or moves it from the register it was returned
in, to the register where tests can happen.  s_strl* then pops the three
arguments to strl*.  Perform the test, push the result of the test, or move it
from the result register to the return value register.  The caller to s_strl*
now has to either pop the return value of s_strl* or move it from the return
value register to the test register.  The caller then pops the three args to
s_strl* off the stack (the same args that s_strl* itself had to pop off after
the real call to strl*).  The s_strl* caller then performs a simular test to
what has already been done, and conditionally jumps.  By doing things this way, we've given the compiler optimizer less to work with.

Also, please don't forget the that call to s_strl* has possibly jumped to code
not in the cache due to being far away from the calling code, thus causing a
pipeline stall.

So where is the "optimization" from s_strl*?
It isn't code clarity.
It isn't code execution speed.  It isn't code size either.

2 files changed, 18 insertions, 32 deletions

diff --git a/usr.sbin/pkg_install/add/main.c b/usr.sbin/pkg_install/add/main.c
index 2844bf9..1c5e4f5 100644
--- a/usr.sbin/pkg_install/add/main.c
+++ b/usr.sbin/pkg_install/add/main.c
@@ -111,7 +111,7 @@ main(int argc, char **argv)
 	    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 @@ main(int argc, char **argv)
     	    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 @@ main(int argc, char **argv)
 		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 @@ getpackagesite(void)
     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;
diff --git a/usr.sbin/pkg_install/lib/str.c b/usr.sbin/pkg_install/lib/str.c
index 55ff782..c5f11a2 100644
--- a/usr.sbin/pkg_install/lib/str.c
+++ b/usr.sbin/pkg_install/lib/str.c
@@ -61,20 +61,6 @@ get_dash_string(char **str)
     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 */
 char *
 copy_string(char *str)