use xmallocz to avoid size arithmetic

We frequently allocate strings as xmalloc(len + 1), where
the extra 1 is for the NUL terminator. This can be done more
simply with xmallocz, which also checks for integer
overflow.

There's no case where switching xmalloc(n+1) to xmallocz(n)
is wrong; the result is the same length, and malloc made no
guarantees about what was in the buffer anyway. But in some
cases, we can stop manually placing NUL at the end of the
allocated buffer. But that's only safe if it's clear that
the contents will always fill the buffer.

In each case where this patch does so, I manually examined
the control flow, and I tried to err on the side of caution.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index fd915d59841ecc1098e2d8a4356947215377d4da..eac499450f63554387fb8b32ef5780c32ded4a21 100644 (file)
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -20,7 +20,7 @@ static const char builtin_check_ref_format_usage[] =
  */
 static char *collapse_slashes(const char *refname)
 {
-       char *ret = xmalloc(strlen(refname) + 1);
+       char *ret = xmallocz(strlen(refname));
        char ch;
        char prev = '/';
        char *cp = ret;

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index d4f0cbd45149eebe15f53ff8c34eefd9b3803d82..ca570041df0f67efa92b056cec01e7d9a4e38ad7 100644 (file)
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -174,7 +174,7 @@ static struct merge_list *create_entry(unsigned stage, unsigned mode, const unsi
 
 static char *traverse_path(const struct traverse_info *info, const struct name_entry *n)
 {
-       char *path = xmalloc(traverse_path_len(info, n) + 1);
+       char *path = xmallocz(traverse_path_len(info, n));
        return make_traverse_path(path, info, n);
 }
 

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 475b9581a5583166c43199f1662b510842c59c7d..0a45710be8561e51d1b2e29586a63b0aa8518e56 100644 (file)
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -52,7 +52,7 @@ static int prune_worktree(const char *id, struct strbuf *reason)
                return 1;
        }
        len = st.st_size;
-       path = xmalloc(len + 1);
+       path = xmallocz(len);
        read_in_full(fd, path, len);
        close(fd);
        while (len && (path[len - 1] == '\n' || path[len - 1] == '\r'))

diff --git a/column.c b/column.c
index f9fda681f13d3bffb44a6d198fe64cc03e4f62bb..d55ead18efeba9435bd2f2e76e2d7ac9ddc064c1 100644 (file)
--- a/column.c
+++ b/column.c
@@ -173,9 +173,8 @@ static void display_table(const struct string_list *list,
        if (colopts & COL_DENSE)
                shrink_columns(&data);
 
-       empty_cell = xmalloc(initial_width + 1);
+       empty_cell = xmallocz(initial_width);
        memset(empty_cell, ' ', initial_width);
-       empty_cell[initial_width] = '\0';
        for (y = 0; y < data.rows; y++) {
                for (x = 0; x < data.cols; x++)
                        if (display_cell(&data, initial_width, empty_cell, x, y))

diff --git a/combine-diff.c b/combine-diff.c
index a698016db7d3eccb77e80219adda60e8b9cccb20..890c4157bdbf69088be359edb9339fbcc15e31bd 100644 (file)
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1043,7 +1043,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
                                elem->mode = canon_mode(S_IFLNK);
 
                        result_size = len;
-                       result = xmalloc(len + 1);
+                       result = xmallocz(len);
 
                        done = read_in_full(fd, result, len);
                        if (done < 0)
@@ -1051,8 +1051,6 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
                        else if (done < len)
                                die("early EOF '%s'", elem->path);
 
-                       result[len] = 0;
-
                        /* If not a fake symlink, apply filters, e.g. autocrlf */
                        if (is_file) {
                                struct strbuf buf = STRBUF_INIT;

diff --git a/config.c b/config.c
index 86a5eb2571fb282a1d7bad9e6d097b2374b550dc..ba8fd1376587848f0691883e7d1d71ad4d75d599 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1878,7 +1878,7 @@ static int git_config_parse_key_1(const char *key, char **store_key, int *basele
         * Validate the key and while at it, lower case it for matching.
         */
        if (store_key)
-               *store_key = xmalloc(strlen(key) + 1);
+               *store_key = xmallocz(strlen(key));
 
        dot = 0;
        for (i = 0; key[i]; i++) {
@@ -1902,8 +1902,6 @@ static int git_config_parse_key_1(const char *key, char **store_key, int *basele
                if (store_key)
                        (*store_key)[i] = c;
        }
-       if (store_key)
-               (*store_key)[i] = 0;
 
        return 0;
 

diff --git a/dir.c b/dir.c
index 363a402887e52a6e551483efa0e1efe3b36f3657..cb5bff8d8f69d05a21fc5a7966aedff975970430 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -711,7 +711,7 @@ static int add_excludes(const char *fname, const char *base, int baselen,
                        close(fd);
                        return 0;
                }
-               buf = xmalloc(size+1);
+               buf = xmallocz(size);
                if (read_in_full(fd, buf, size) != size) {
                        free(buf);
                        close(fd);

diff --git a/entry.c b/entry.c
index 582c40071a3034af9a391cf438dd74ddb34e15ff..a4109574fa72e0f3f32ad7b9e1ed0e402c8aecff 100644 (file)
--- a/entry.c
+++ b/entry.c
@@ -6,7 +6,7 @@
 static void create_directories(const char *path, int path_len,
                               const struct checkout *state)
 {
-       char *buf = xmalloc(path_len + 1);
+       char *buf = xmallocz(path_len);
        int len = 0;
 
        while (len < path_len) {

diff --git a/grep.c b/grep.c
index 7b2b96a4376efe51e41f9311c6fa5250a2dcd13d..528b652f713d2b6db5f48e3829448212cc3837bf 100644 (file)
--- a/grep.c
+++ b/grep.c
@@ -1741,7 +1741,7 @@ static int grep_source_load_file(struct grep_source *gs)
        i = open(filename, O_RDONLY);
        if (i < 0)
                goto err_ret;
-       data = xmalloc(size + 1);
+       data = xmallocz(size);
        if (st.st_size != read_in_full(i, data, size)) {
                error(_("'%s': short read %s"), filename, strerror(errno));
                close(i);
@@ -1749,7 +1749,6 @@ static int grep_source_load_file(struct grep_source *gs)
                return -1;
        }
        close(i);
-       data[size] = 0;
 
        gs->buf = data;
        gs->size = size;

diff --git a/imap-send.c b/imap-send.c
index 4d3b7737a99de02c42945951e91b4d7ed865611d..2c52027c84455819740bed9f53970e175b9ca133 100644 (file)
--- a/imap-send.c
+++ b/imap-send.c
@@ -892,12 +892,11 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)
        response = xstrfmt("%s %s", user, hex);
        resp_len = strlen(response) + 1;
 
-       response_64 = xmalloc(ENCODED_SIZE(resp_len) + 1);
+       response_64 = xmallocz(ENCODED_SIZE(resp_len));
        encoded_len = EVP_EncodeBlock((unsigned char *)response_64,
                                      (unsigned char *)response, resp_len);
        if (encoded_len < 0)
                die("EVP_EncodeBlock error");
-       response_64[encoded_len] = '\0';
        return (char *)response_64;
 }
 
@@ -1188,7 +1187,7 @@ static void lf_to_crlf(struct strbuf *msg)
                j++;
        }
 
-       new = xmalloc(j + 1);
+       new = xmallocz(j);
 
        /*
         * Second pass: write the new string.  Note that this loop is

diff --git a/ll-merge.c b/ll-merge.c
index 0338630fc2a5378ed33afa828db4940b34aabe82..ff4a43a982a6a67ec826fdb01153a9e35e720751 100644 (file)
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -205,7 +205,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
        if (fstat(fd, &st))
                goto close_bad;
        result->size = st.st_size;
-       result->ptr = xmalloc(result->size + 1);
+       result->ptr = xmallocz(result->size);
        if (read_in_full(fd, result->ptr, result->size) != result->size) {
                free(result->ptr);
                result->ptr = NULL;

diff --git a/progress.c b/progress.c
index 353bd37416e65f6dba733ac9d14999f1e293b4b9..76a88c573f7895bbf388ab4335faa1b86c3975d3 100644 (file)
--- a/progress.c
+++ b/progress.c
@@ -247,7 +247,7 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
                size_t len = strlen(msg) + 5;
                struct throughput *tp = progress->throughput;
 
-               bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);
+               bufp = (len < sizeof(buf)) ? buf : xmallocz(len);
                if (tp) {
                        unsigned int rate = !tp->avg_misecs ? 0 :
                                        tp->avg_bytes / tp->avg_misecs;

diff --git a/refs.c b/refs.c
index e2d34b253e4c9c4efcb30e88d11ccd3ce9e14deb..1d9e2a7932413a31fde81512ddcfdb7655209b1e 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -124,7 +124,7 @@ int refname_is_safe(const char *refname)
                char *buf;
                int result;
 
-               buf = xmalloc(strlen(refname) + 1);
+               buf = xmallocz(strlen(refname));
                /*
                 * Does the refname try to escape refs/?
                 * For example: refs/foo/../bar is safe but refs/foo/../../bar

diff --git a/setup.c b/setup.c
index 2c4b22c8456733a3ab2848fff81630589139c671..669062a0906ce7a78810582332a2b6bb72c3d0c5 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -88,7 +88,7 @@ char *prefix_path_gently(const char *prefix, int len,
        const char *orig = path;
        char *sanitized;
        if (is_absolute_path(orig)) {
-               sanitized = xmalloc(strlen(path) + 1);
+               sanitized = xmallocz(strlen(path));
                if (remaining_prefix)
                        *remaining_prefix = 0;
                if (normalize_path_copy_len(sanitized, path, remaining_prefix)) {
@@ -499,14 +499,13 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
                error_code = READ_GITFILE_ERR_OPEN_FAILED;
                goto cleanup_return;
        }
-       buf = xmalloc(st.st_size + 1);
+       buf = xmallocz(st.st_size);
        len = read_in_full(fd, buf, st.st_size);
        close(fd);
        if (len != st.st_size) {
                error_code = READ_GITFILE_ERR_READ_FAILED;
                goto cleanup_return;
        }
-       buf[len] = '\0';
        if (!starts_with(buf, "gitdir: ")) {
                error_code = READ_GITFILE_ERR_INVALID_FORMAT;
                goto cleanup_return;

diff --git a/strbuf.c b/strbuf.c
index d76f0aed85c4ec6eafdcfd1a8ee2b22d3d20df96..de7a7c273095e4f5907260ddda50bef375381783 100644 (file)
--- a/strbuf.c
+++ b/strbuf.c
@@ -685,7 +685,7 @@ char *xstrdup_tolower(const char *string)
        size_t len, i;
 
        len = strlen(string);
-       result = xmalloc(len + 1);
+       result = xmallocz(len);
        for (i = 0; i < len; i++)
                result[i] = tolower(string[i]);
        result[i] = '\0';