replace trivial malloc + sprintf / strcpy calls with xstrfmt
authorJeff King <peff@peff.net>
Thu, 24 Sep 2015 21:07:03 +0000 (17:07 -0400)
committerJunio C Hamano <gitster@pobox.com>
Fri, 25 Sep 2015 17:18:18 +0000 (10:18 -0700)
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>
builtin/apply.c
builtin/ls-remote.c
builtin/name-rev.c
environment.c
imap-send.c
reflog-walk.c
remote.c
setup.c
unpack-trees.c

index 4aa53f7..094a20f 100644 (file)
@@ -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);
        }
 
index 4554dbc..5b6d679 100644 (file)
@@ -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) {
index 248a3eb..8a3a0cd 100644 (file)
@@ -56,19 +56,16 @@ copy_data:
                        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);
index a533aed..c5b65f5 100644 (file)
@@ -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);
index 37ac4aa..e9faaea 100644 (file)
@@ -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,
index f8e743a..85b8a54 100644 (file)
@@ -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);
index 26504b7..5ab0f7f 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 a17c51e..2b64cbb 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;
        }
index f932e80..8e2032f 100644 (file)
@@ -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)