replace trivial malloc + sprintf / strcpy calls with xstrfmt

It's a common pattern to do:

  foo = xmalloc(strlen(one) + strlen(two) + 1 + 1);
  sprintf(foo, "%s %s", one, two);

(or possibly some variant with strcpy()s or a more
complicated length computation).  We can switch these to use
xstrfmt, which is shorter, involves less error-prone manual
computation, and removes many sprintf and strcpy calls which
make it harder to audit the code for real buffer overflows.

Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: peff

builtin/apply.c

@@ -698,10 +698,7 @@ static char *find_name_common(const char *line, const char *def,
 	}
 
 	if (root) {
-		char *ret = xmalloc(root_len + len + 1);
-		strcpy(ret, root);
-		memcpy(ret + root_len, start, len);
-		ret[root_len + len] = '\0';
+		char *ret = xstrfmt("%s%.*s", root, len, start);
 		return squash_slash(ret);
 	}
 

builtin/ls-remote.c

@@ -93,12 +93,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	if (argv[i]) {
 		int j;
 		pattern = xcalloc(argc - i + 1, sizeof(const char *));
-		for (j = i; j < argc; j++) {
-			int len = strlen(argv[j]);
-			char *p = xmalloc(len + 3);
-			sprintf(p, "*/%s", argv[j]);
-			pattern[j - i] = p;
-		}
+		for (j = i; j < argc; j++)
+			pattern[j - i] = xstrfmt("*/%s", argv[j]);
 	}
 	remote = remote_get(dest);
 	if (!remote) {

builtin/name-rev.c

@@ -56,19 +56,16 @@ static void name_rev(struct commit *commit,
 			parents = parents->next, parent_number++) {
 		if (parent_number > 1) {
 			int len = strlen(tip_name);
-			char *new_name = xmalloc(len +
-				1 + decimal_length(generation) +  /* ~<n> */
-				1 + 2 +				  /* ^NN */
-				1);
+			char *new_name;
 
 			if (len > 2 && !strcmp(tip_name + len - 2, "^0"))
 				len -= 2;
 			if (generation > 0)
-				sprintf(new_name, "%.*s~%d^%d", len, tip_name,
-						generation, parent_number);
+				new_name = xstrfmt("%.*s~%d^%d", len, tip_name,
+						   generation, parent_number);
 			else
-				sprintf(new_name, "%.*s^%d", len, tip_name,
-						parent_number);
+				new_name = xstrfmt("%.*s^%d", len, tip_name,
+						   parent_number);
 
 			name_rev(parents->item, new_name, 0,
 				distance + MERGE_TRAVERSAL_WEIGHT, 0);

environment.c

@@ -143,11 +143,8 @@ static char *git_path_from_env(const char *envvar, const char *git_dir,
 			       const char *path, int *fromenv)
 {
 	const char *value = getenv(envvar);
-	if (!value) {
-		char *buf = xmalloc(strlen(git_dir) + strlen(path) + 2);
-		sprintf(buf, "%s/%s", git_dir, path);
-		return buf;
-	}
+	if (!value)
+		return xstrfmt("%s/%s", git_dir, path);
 	if (fromenv)
 		*fromenv = 1;
 	return xstrdup(value);

imap-send.c

@@ -889,9 +889,8 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)
 	}
 
 	/* response: "<user> <digest in hex>" */
-	resp_len = strlen(user) + 1 + strlen(hex) + 1;
-	response = xmalloc(resp_len);
-	sprintf(response, "%s %s", user, hex);
+	response = xstrfmt("%s %s", user, hex);
+	resp_len = strlen(response) + 1;
 
 	response_64 = xmalloc(ENCODED_SIZE(resp_len) + 1);
 	encoded_len = EVP_EncodeBlock((unsigned char *)response_64,

reflog-walk.c

@@ -56,12 +56,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
 		}
 	}
 	if (reflogs->nr == 0) {
-		int len = strlen(ref);
-		char *refname = xmalloc(len + 12);
-		sprintf(refname, "refs/%s", ref);
+		char *refname = xstrfmt("refs/%s", ref);
 		for_each_reflog_ent(refname, read_one_reflog, reflogs);
 		if (reflogs->nr == 0) {
-			sprintf(refname, "refs/heads/%s", ref);
+			free(refname);
+			refname = xstrfmt("refs/heads/%s", ref);
 			for_each_reflog_ent(refname, read_one_reflog, reflogs);
 		}
 		free(refname);

remote.c

@@ -65,7 +65,6 @@ static int valid_remote(const struct remote *remote)
 static const char *alias_url(const char *url, struct rewrites *r)
 {
 	int i, j;
-	char *ret;
 	struct counted_string *longest;
 	int longest_i;
 
@@ -86,11 +85,7 @@ static const char *alias_url(const char *url, struct rewrites *r)
 	if (!longest)
 		return url;
 
-	ret = xmalloc(r->rewrite[longest_i]->baselen +
-		     (strlen(url) - longest->len) + 1);
-	strcpy(ret, r->rewrite[longest_i]->base);
-	strcpy(ret + r->rewrite[longest_i]->baselen, url + longest->len);
-	return ret;
+	return xstrfmt("%s%s", r->rewrite[longest_i]->base, url + longest->len);
 }
 
 static void add_push_refspec(struct remote *remote, const char *ref)

setup.c

@@ -99,10 +99,7 @@ char *prefix_path_gently(const char *prefix, int len,
 			return NULL;
 		}
 	} else {
-		sanitized = xmalloc(len + strlen(path) + 1);
-		if (len)
-			memcpy(sanitized, prefix, len);
-		strcpy(sanitized + len, path);
+		sanitized = xstrfmt("%.*s%s", len, prefix, path);
 		if (remaining_prefix)
 			*remaining_prefix = len;
 		if (normalize_path_copy_len(sanitized, sanitized, remaining_prefix)) {
@@ -468,11 +465,8 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
 
 	if (!is_absolute_path(dir) && (slash = strrchr(path, '/'))) {
 		size_t pathlen = slash+1 - path;
-		size_t dirlen = pathlen + len - 8;
-		dir = xmalloc(dirlen + 1);
-		strncpy(dir, path, pathlen);
-		strncpy(dir + pathlen, buf + 8, len - 8);
-		dir[dirlen] = '\0';
+		dir = xstrfmt("%.*s%.*s", (int)pathlen, path,
+			      (int)(len - 8), buf + 8);
 		free(buf);
 		buf = dir;
 	}

unpack-trees.c

@@ -1350,9 +1350,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
 	 * Then we need to make sure that we do not lose a locally
 	 * present file that is not ignored.
 	 */
-	pathbuf = xmalloc(namelen + 2);
-	memcpy(pathbuf, ce->name, namelen);
-	strcpy(pathbuf+namelen, "/");
+	pathbuf = xstrfmt("%.*s/", namelen, ce->name);
 
 	memset(&d, 0, sizeof(d));
 	if (o->dir)