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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 24, 2015 at 17:07 UTC 75faa45ae0230b321bf72027b2274315d7e14e34
9 files changed +20 -48
builtin/apply.c
+1 -4
@@ -698,10 +698,7 @@ static char *find_name_common(const char *line, const char *def,
698 }
699
700 if (root) {
701 - char *ret = xmalloc(root_len + len + 1);
702 - strcpy(ret, root);
703 - memcpy(ret + root_len, start, len);
704 - ret[root_len + len] = '\0';
701 + char *ret = xstrfmt("%s%.*s", root, len, start);
702 return squash_slash(ret);
703 }
704
builtin/ls-remote.c
+2 -6
@@ -93,12 +93,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
93 if (argv[i]) {
94 int j;
95 pattern = xcalloc(argc - i + 1, sizeof(const char *));
96 - for (j = i; j < argc; j++) {
97 - int len = strlen(argv[j]);
98 - char *p = xmalloc(len + 3);
99 - sprintf(p, "*/%s", argv[j]);
100 - pattern[j - i] = p;
101 - }
96 + for (j = i; j < argc; j++)
97 + pattern[j - i] = xstrfmt("*/%s", argv[j]);
98 }
99 remote = remote_get(dest);
100 if (!remote) {
builtin/name-rev.c
+5 -8
@@ -56,19 +56,16 @@ copy_data:
56 parents = parents->next, parent_number++) {
57 if (parent_number > 1) {
58 int len = strlen(tip_name);
59 - char *new_name = xmalloc(len +
60 - 1 + decimal_length(generation) + /* ~<n> */
61 - 1 + 2 + /* ^NN */
62 - 1);
59 + char *new_name;
60
61 if (len > 2 && !strcmp(tip_name + len - 2, "^0"))
62 len -= 2;
63 if (generation > 0)
67 - sprintf(new_name, "%.*s~%d^%d", len, tip_name,
68 - generation, parent_number);
64 + new_name = xstrfmt("%.*s~%d^%d", len, tip_name,
65 + generation, parent_number);
66 else
70 - sprintf(new_name, "%.*s^%d", len, tip_name,
71 - parent_number);
67 + new_name = xstrfmt("%.*s^%d", len, tip_name,
68 + parent_number);
69
70 name_rev(parents->item, new_name, 0,
71 distance + MERGE_TRAVERSAL_WEIGHT, 0);
environment.c
+2 -5
@@ -143,11 +143,8 @@ static char *git_path_from_env(const char *envvar, const char *git_dir,
143 const char *path, int *fromenv)
144 {
145 const char *value = getenv(envvar);
146 - if (!value) {
147 - char *buf = xmalloc(strlen(git_dir) + strlen(path) + 2);
148 - sprintf(buf, "%s/%s", git_dir, path);
149 - return buf;
150 - }
146 + if (!value)
147 + return xstrfmt("%s/%s", git_dir, path);
148 if (fromenv)
149 *fromenv = 1;
150 return xstrdup(value);
imap-send.c
+2 -3
@@ -889,9 +889,8 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)
889 }
890
891 /* response: "<user> <digest in hex>" */
892 - resp_len = strlen(user) + 1 + strlen(hex) + 1;
893 - response = xmalloc(resp_len);
894 - sprintf(response, "%s %s", user, hex);
892 + response = xstrfmt("%s %s", user, hex);
893 + resp_len = strlen(response) + 1;
894
895 response_64 = xmalloc(ENCODED_SIZE(resp_len) + 1);
896 encoded_len = EVP_EncodeBlock((unsigned char *)response_64,
reflog-walk.c
+3 -4
@@ -56,12 +56,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
56 }
57 }
58 if (reflogs->nr == 0) {
59 - int len = strlen(ref);
60 - char *refname = xmalloc(len + 12);
61 - sprintf(refname, "refs/%s", ref);
59 + char *refname = xstrfmt("refs/%s", ref);
60 for_each_reflog_ent(refname, read_one_reflog, reflogs);
61 if (reflogs->nr == 0) {
64 - sprintf(refname, "refs/heads/%s", ref);
62 + free(refname);
63 + refname = xstrfmt("refs/heads/%s", ref);
64 for_each_reflog_ent(refname, read_one_reflog, reflogs);
65 }
66 free(refname);
remote.c
+1 -6
@@ -65,7 +65,6 @@ static int valid_remote(const struct remote *remote)
65 static const char *alias_url(const char *url, struct rewrites *r)
66 {
67 int i, j;
68 - char *ret;
68 struct counted_string *longest;
69 int longest_i;
70
@@ -86,11 +85,7 @@ static const char *alias_url(const char *url, struct rewrites *r)
85 if (!longest)
86 return url;
87
89 - ret = xmalloc(r->rewrite[longest_i]->baselen +
90 - (strlen(url) - longest->len) + 1);
91 - strcpy(ret, r->rewrite[longest_i]->base);
92 - strcpy(ret + r->rewrite[longest_i]->baselen, url + longest->len);
93 - return ret;
88 + return xstrfmt("%s%s", r->rewrite[longest_i]->base, url + longest->len);
89 }
90
91 static void add_push_refspec(struct remote *remote, const char *ref)
setup.c
+3 -9
@@ -99,10 +99,7 @@ char *prefix_path_gently(const char *prefix, int len,
99 return NULL;
100 }
101 } else {
102 - sanitized = xmalloc(len + strlen(path) + 1);
103 - if (len)
104 - memcpy(sanitized, prefix, len);
105 - strcpy(sanitized + len, path);
102 + sanitized = xstrfmt("%.*s%s", len, prefix, path);
103 if (remaining_prefix)
104 *remaining_prefix = len;
105 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)
465
466 if (!is_absolute_path(dir) && (slash = strrchr(path, '/'))) {
467 size_t pathlen = slash+1 - path;
471 - size_t dirlen = pathlen + len - 8;
472 - dir = xmalloc(dirlen + 1);
473 - strncpy(dir, path, pathlen);
474 - strncpy(dir + pathlen, buf + 8, len - 8);
475 - dir[dirlen] = '\0';
468 + dir = xstrfmt("%.*s%.*s", (int)pathlen, path,
469 + (int)(len - 8), buf + 8);
470 free(buf);
471 buf = dir;
472 }
unpack-trees.c
+1 -3
@@ -1350,9 +1350,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
1350 * Then we need to make sure that we do not lose a locally
1351 * present file that is not ignored.
1352 */
1353 - pathbuf = xmalloc(namelen + 2);
1354 - memcpy(pathbuf, ce->name, namelen);
1355 - strcpy(pathbuf+namelen, "/");
1353 + pathbuf = xstrfmt("%.*s/", namelen, ce->name);
1354
1355 memset(&d, 0, sizeof(d));
1356 if (o->dir)