Clean up some of the evil string handling in this code, replace

warnx()+exit() with errx() and replace a big if..then..else construct
to determine the package download directory with a lookup table.

Reviewed by:	jkh
MFC after:	2 weeks

usr.sbin/pkg_install/add/main.c

@@ -48,6 +48,23 @@ add_mode_t AddMode	= NORMAL;
 char	pkgnames[MAX_PKGS][MAXPATHLEN];
 char	*pkgs[MAX_PKGS];
 
+struct {
+	int lowver;	/* Lowest version number to match */
+	int hiver;	/* Highest version number to match */
+	const char *directory;	/* Directory it lives in */
+} releases[] = {
+	{ 410000, 410000, "/packages-4.1-release" },
+	{ 420000, 420000, "/packages-4.2-release" },
+	{ 430000, 430000, "/packages-4.3-release" },
+	{ 440000, 440000, "/packages-4.4-release" },
+	{ 450000, 450000, "/packages-4.5-release" },
+	{ 300000, 399000, "/packages-3-stable" },
+	{ 400000, 499000, "/packages-4-stable" },
+	{ 510000, 599000, "/packages-5-stable" },
+	{ 0, 9999999, "/packages-current" },
+	{ 0, 0, NULL }
+};
+
 static char *getpackagesite(void);
 int getosreldate(void);
 
@@ -56,11 +73,9 @@ static void usage __P((void));
 int
 main(int argc, char **argv)
 {
-    int ch, err;
+    int ch, error;
     char **start;
-    char *cp;
-
-    char *remotepkg = NULL, *ptr;
+    char *cp, *packagesite, *remotepkg = NULL, *ptr;
     static char temppackageroot[MAXPATHLEN];
 
     start = argv;
@@ -96,7 +111,8 @@ main(int argc, char **argv)
 	    break;
 
 	case 't':
-	    strcpy(FirstPen, optarg);
+	    if (s_strlcpy(FirstPen, optarg, sizeof(FirstPen)))
+		errx(1, "-t Argument too long.");
 	    break;
 
 	case 'S':
@@ -118,8 +134,7 @@ main(int argc, char **argv)
     argv += optind;
 
     if (argc > MAX_PKGS) {
-	warnx("too many packages (max %d)", MAX_PKGS);
-	return(1);
+	errx(1, "too many packages (max %d)", MAX_PKGS);
     }
 
     if (AddMode != SLAVE) {
@@ -128,26 +143,42 @@ main(int argc, char **argv)
 	/* Get all the remaining package names, if any */
 	for (ch = 0; *argv; ch++, argv++) {
     	    if (Remote) {
-		strcpy(temppackageroot, getpackagesite());
-		remotepkg = strcat(temppackageroot, *argv);
+		if ((packagesite = getpackagesite()) == NULL)
+		    errx(1, "package name too long");
+		if (s_strlcpy(temppackageroot, packagesite,
+		    sizeof(temppackageroot)))
+		    errx(1, "package name too long");
+		if (s_strlcat(temppackageroot, *argv,
+		    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]))
-		   strcat(remotepkg, ".tgz");
+		    if (s_strlcat(remotepkg, ".tgz", sizeof(temppackageroot)))
+			errx(1, "package name too long");
     	    }
 	    if (!strcmp(*argv, "-"))	/* stdin? */
 		pkgs[ch] = "-";
-	    else if (isURL(*argv))	/* preserve URLs */
-		pkgs[ch] = strcpy(pkgnames[ch], *argv);
-	    else if ((Remote) && isURL(remotepkg))
-		pkgs[ch] = strcpy(pkgnames[ch], remotepkg);
-	    else {			/* expand all pathnames to fullnames */
+	    else if (isURL(*argv)) {  	/* preserve URLs */
+		if (s_strlcpy(pkgnames[ch], *argv, 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])))
+		    errx(1, "package name too long");
+		pkgs[ch] = pkgnames[ch];
+	    } else {			/* expand all pathnames to fullnames */
 		if (fexists(*argv)) /* refers to a file directly */
 		    pkgs[ch] = realpath(*argv, pkgnames[ch]);
 		else {		/* look for the file in the expected places */
 		    if (!(cp = fileFindByPath(NULL, *argv)))
 			warnx("can't find package '%s'", *argv);
-		    else
-			pkgs[ch] = strcpy(pkgnames[ch], cp);
+		    else {
+			if (s_strlcpy(pkgnames[ch], cp, sizeof(pkgnames[ch])))
+			    errx(1, "package name too long");
+			pkgs[ch] = pkgnames[ch];
+		    }
 		}
 	    }
 	}
@@ -169,10 +200,10 @@ main(int argc, char **argv)
     /* Set a reasonable umask */
     umask(022);
 
-    if ((err = pkg_perform(pkgs)) != 0) {
+    if ((error = pkg_perform(pkgs)) != 0) {
 	if (Verbose)
-	    warnx("%d package addition(s) failed", err);
-	return err;
+	    warnx("%d package addition(s) failed", error);
+	return error;
     }
     else
 	return 0;
@@ -181,46 +212,43 @@ main(int argc, char **argv)
 static char *
 getpackagesite(void)
 {
-    int reldate;
+    int reldate, i;
     static char sitepath[MAXPATHLEN];
     struct utsname u;
 
     if (getenv("PACKAGESITE")) {
-	strcpy(sitepath, getenv("PACKAGESITE"));
+	if (s_strlcpy(sitepath, getenv("PACKAGESITE"), 
+	    sizeof(sitepath)))
+	    return NULL;
 	return sitepath;
     }
 
-    if (getenv("PACKAGEROOT"))
-	strcpy(sitepath, getenv("PACKAGEROOT"));
-    else
-	strcpy(sitepath, "ftp://ftp.FreeBSD.org");
+    if (getenv("PACKAGEROOT")) {
+	if (s_strlcpy(sitepath, getenv("PACKAGEROOT"), sizeof(sitepath)))
+	    return NULL;
+    } else {
+	if (s_strlcat(sitepath, "ftp://ftp.freebsd.org", sizeof(sitepath)))
+	    return NULL;
+    }
 
-    strcat(sitepath, "/pub/FreeBSD/ports/");
+    if (s_strlcat(sitepath, "/pub/FreeBSD/ports/", sizeof(sitepath)))
+	return NULL;
 
     uname(&u);
-    strcat(sitepath, u.machine);
+    if (s_strlcat(sitepath, u.machine, sizeof(sitepath)))
+	return NULL;
 
     reldate = getosreldate();
-    if (reldate == 410000)
-	strcat(sitepath, "/packages-4.1-release");
-    else if (reldate == 420000)
-	strcat(sitepath, "/packages-4.2-release");
-    else if (reldate == 430000)
-	strcat(sitepath, "/packages-4.3-release");
-    else if (reldate == 440000)
-	strcat(sitepath, "/packages-4.4-release");
-    else if (reldate == 450000)
-	strcat(sitepath, "/packages-4.5-release");
-    else if (300000 <= reldate && reldate <= 399000)
-	strcat(sitepath, "/packages-3-stable");
-    else if (400000 <= reldate && reldate <= 499000)
-	strcat(sitepath, "/packages-4-stable");
-    else if (510000 <= reldate && reldate <= 599000)	/* get real values!! */
-	strcat(sitepath, "/packages-5-stable");
-    else
-	strcat(sitepath, "/packages-current");
+    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)))
+		return NULL;
+	    continue;
+	}
+    }
 
-    strcat(sitepath, "/Latest/");
+    if (s_strlcat(sitepath, "/Latest/", sizeof(sitepath)))
+	return NULL;
 
     return sitepath;
 

usr.sbin/pkg_install/lib/lib.h

@@ -128,6 +128,8 @@ void		str_lowercase(char *);
 char		*basename_of(char *);
 char		*strconcat(char *, char *);
 char		*get_string(char *, int, FILE *);
+int		s_strlcpy(char *, const char *, size_t);
+int		s_strlcat(char *, const char *, size_t);
 
 /* File */
 Boolean		fexists(char *);

usr.sbin/pkg_install/lib/str.c

@@ -61,6 +61,20 @@ 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)