[pacman-dev] [PATCH 1/2] Remove most usages of strncmp()

[Dan McGee]

The supposed safety blanket of this function is better handled by
explicit length checking and usages of strlen() on known NULL-terminated
strings rather than hoping things fit in a buffer. We also have no need
to fully fill a PATH_MAX length variable with NULLs every time as long
as a single terminating byte is there. Remove usages of it by using
strcpy() or memcpy() as appropriate, after doing length checks via
strlen().

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/handle.c |    2 +-
 src/pacman/query.c   |   17 ++++++++++-------
 src/pacman/util.c    |   11 ++++-------
 src/util/vercmp.c    |    2 +-
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index 22f3fc8..b48d5b6 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -298,7 +298,7 @@ static char *canonicalize_path(const char *path) {
 		len += 1;
 	}
 	CALLOC(new_path, len + 1, sizeof(char), return NULL);
-	strncpy(new_path, path, len);
+	strcpy(new_path, path);
 	new_path[len - 1] = '/';
 	return new_path;
 }
diff --git a/src/pacman/query.c b/src/pacman/query.c
index 045dc7f..4fc4584 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -110,8 +110,7 @@ static int query_fileowner(alpm_list_t *targets)
 	int ret = 0;
 	char path[PATH_MAX];
 	const char *root;
-	char *append;
-	size_t max_length;
+	size_t rootlen;
 	alpm_list_t *t;
 	alpm_db_t *db_local;
 
@@ -125,9 +124,13 @@ static int query_fileowner(alpm_list_t *targets)
 	 * once, then we can just overwrite whatever file was there on the previous
 	 * iteration. */
 	root = alpm_option_get_root(config->handle);
-	strncpy(path, root, PATH_MAX - 1);
-	append = path + strlen(path);
-	max_length = PATH_MAX - (append - path) - 1;
+	rootlen = strlen(root);
+	if(rootlen + 1 > PATH_MAX) {
+		/* we are in trouble here */
+		pm_fprintf(stderr, ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, "");
+		return 1;
+	}
+	strcpy(path, root);
 
 	db_local = alpm_option_get_localdb(config->handle);
 
@@ -208,11 +211,11 @@ static int query_fileowner(alpm_list_t *targets)
 					continue;
 				}
 
-				if(strlen(pkgfile) > max_length) {
+				if(rootlen + 1 + strlen(pkgfile) > PATH_MAX) {
 					pm_fprintf(stderr, ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, pkgfile);
 				}
 				/* concatenate our file and the root path */
-				strcpy(append, pkgfile);
+				strcpy(path + rootlen, pkgfile);
 
 				pdname = mdirname(path);
 				ppath = resolve_path(pdname);
diff --git a/src/pacman/util.c b/src/pacman/util.c
index 9ced7aa..203f927 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -361,22 +361,21 @@ char *strreplace(const char *str, const char *needle, const char *replace)
 	 * x "size difference between replace and needle" */
 	newsz = strlen(str) + 1 +
 		alpm_list_count(list) * (replacesz - needlesz);
-	newstr = malloc(newsz);
+	newstr = calloc(newsz, sizeof(char));
 	if(!newstr) {
 		return NULL;
 	}
-	*newstr = '\0';
 
 	p = str;
 	newp = newstr;
 	for(i = list; i; i = alpm_list_next(i)) {
 		q = alpm_list_getdata(i);
-		if(q > p){
+		if(q > p) {
 			/* add chars between this occurence and last occurence, if any */
-			strncpy(newp, p, (size_t)(q - p));
+			memcpy(newp, p, (size_t)(q - p));
 			newp += q - p;
 		}
-		strncpy(newp, replace, replacesz);
+		memcpy(newp, replace, replacesz);
 		newp += replacesz;
 		p = q + needlesz;
 	}
@@ -385,9 +384,7 @@ char *strreplace(const char *str, const char *needle, const char *replace)
 	if(*p) {
 		/* add the rest of 'p' */
 		strcpy(newp, p);
-		newp += strlen(p);
 	}
-	*newp = '\0';
 
 	return newstr;
 }
diff --git a/src/util/vercmp.c b/src/util/vercmp.c
index 88cf49a..f4356fb 100644
--- a/src/util/vercmp.c
+++ b/src/util/vercmp.c
@@ -20,7 +20,7 @@
 
 #include <stdlib.h>
 #include <stdio.h> /* printf */
-#include <string.h> /* strncpy */
+#include <string.h>
 
 #define BASENAME "vercmp"
 
-- 
1.7.6