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>

diff --git a/builtin/apply.c b/builtin/apply.c
index 4aa53f7fd87d5fd23c0929360b6746f48e8e3e73..094a20f4896fa0a29d10e4fa33c656ea53616bfc 100644 (file)
--- a/builtin/apply.c
+++ b/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);
        }
 

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 4554dbc8a98c0daaa67c8ea65f5ddf5c48f19383..5b6d679a639d2376fe24f3ff748d3853696700b2 100644 (file)
--- a/builtin/ls-remote.c
+++ b/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) {

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 248a3eb260cfa22a11d4b1b5420c42bc608f13c5..8a3a0cd61ef40e4ebff872188f5938dcd700870c 100644 (file)
--- a/builtin/name-rev.c
+++ b/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);

diff --git a/environment.c b/environment.c
index a533aed630c20a5e0718bcea4375875a44896416..c5b65f5e231e02f315d8a24ee48034aede3ad5e8 100644 (file)
--- a/environment.c
+++ b/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);

diff --git a/imap-send.c b/imap-send.c
index 37ac4aa86a740ecc8430afff3b9d3c7a0c753a34..e9faaeaf2ab31413139dad17c6df1032e7e3cffa 100644 (file)
--- a/imap-send.c
+++ b/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,

diff --git a/reflog-walk.c b/reflog-walk.c
index f8e743a23bef06bbf144339dc79f2186ec40805b..85b8a54241048bb2cc037c2c114b49e7c7e7968b 100644 (file)
--- a/reflog-walk.c
+++ b/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);

diff --git a/remote.c b/remote.c
index 26504b744786c65ea4d6e1e0abbf5c6409af5358..5ab0f7f7a5443a0f8aeaf92ab13567740862e5e4 100644 (file)
--- a/remote.c
+++ b/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)

diff --git a/setup.c b/setup.c
index a17c51e61d75ac8280bf04d95c50d7bdfd6d7a0e..2b64cbbbfac60ac2db169ea19c03c891f0333864 100644 (file)
--- a/setup.c
+++ b/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;
        }

diff --git a/unpack-trees.c b/unpack-trees.c
index f932e80e862cfafb7b909059b460dcb688c6dc0b..8e2032f4e592910d6f3336f95019647992113877 100644 (file)
--- a/unpack-trees.c
+++ b/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)