@matheusbsbs / matheus-bagliano / commits / 3733e69464

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>

Jeff King committed Feb 22, 2016 at 17:44 UTC 3733e6946465d4a3a1d89026a5ec911d3af339ab

15 files changed +17 -25

builtin/check-ref-format.c

+1 -1

@@ -20,7 +20,7 @@ static const char builtin_check_ref_format_usage[] =

20 */

21 static char *collapse_slashes(const char *refname)

22 {

23 - char *ret = xmalloc(strlen(refname) + 1);

23 + char *ret = xmallocz(strlen(refname));

24 char ch;

25 char prev = '/';

26 char *cp = ret;

builtin/merge-tree.c

+1 -1

@@ -174,7 +174,7 @@ static struct merge_list *create_entry(unsigned stage, unsigned mode, const unsi

174

175 static char *traverse_path(const struct traverse_info *info, const struct name_entry *n)

176 {

177 - char *path = xmalloc(traverse_path_len(info, n) + 1);

177 + char *path = xmallocz(traverse_path_len(info, n));

178 return make_traverse_path(path, info, n);

179 }

180

builtin/worktree.c

+1 -1

@@ -52,7 +52,7 @@ static int prune_worktree(const char *id, struct strbuf *reason)

52 return 1;

53 }

54 len = st.st_size;

55 - path = xmalloc(len + 1);

55 + path = xmallocz(len);

56 read_in_full(fd, path, len);

57 close(fd);

58 while (len && (path[len - 1] == '\n' || path[len - 1] == '\r'))

column.c

+1 -2

@@ -173,9 +173,8 @@ static void display_table(const struct string_list *list,

173 if (colopts & COL_DENSE)

174 shrink_columns(&data);

175

176 - empty_cell = xmalloc(initial_width + 1);

176 + empty_cell = xmallocz(initial_width);

177 memset(empty_cell, ' ', initial_width);

178 - empty_cell[initial_width] = '\0';

178 for (y = 0; y < data.rows; y++) {

179 for (x = 0; x < data.cols; x++)

180 if (display_cell(&data, initial_width, empty_cell, x, y))

combine-diff.c

+1 -3

@@ -1043,7 +1043,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,

1043 elem->mode = canon_mode(S_IFLNK);

1044

1045 result_size = len;

1046 - result = xmalloc(len + 1);

1046 + result = xmallocz(len);

1047

1048 done = read_in_full(fd, result, len);

1049 if (done < 0)

@@ -1051,8 +1051,6 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,

1051 else if (done < len)

1052 die("early EOF '%s'", elem->path);

1053

1054 - result[len] = 0;

1055 -

1054 /* If not a fake symlink, apply filters, e.g. autocrlf */

1055 if (is_file) {

1056 struct strbuf buf = STRBUF_INIT;

config.c

+1 -3

@@ -1878,7 +1878,7 @@ static int git_config_parse_key_1(const char *key, char **store_key, int *basele

1878 * Validate the key and while at it, lower case it for matching.

1879 */

1880 if (store_key)

1881 - *store_key = xmalloc(strlen(key) + 1);

1881 + *store_key = xmallocz(strlen(key));

1882

1883 dot = 0;

1884 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

1902 if (store_key)

1903 (*store_key)[i] = c;

1904 }

1905 - if (store_key)

1906 - (*store_key)[i] = 0;

1905

1906 return 0;

1907

dir.c

+1 -1

@@ -711,7 +711,7 @@ static int add_excludes(const char *fname, const char *base, int baselen,

711 close(fd);

712 return 0;

713 }

714 - buf = xmalloc(size+1);

714 + buf = xmallocz(size);

715 if (read_in_full(fd, buf, size) != size) {

716 free(buf);

717 close(fd);

entry.c

+1 -1

@@ -6,7 +6,7 @@

6 static void create_directories(const char *path, int path_len,

7 const struct checkout *state)

8 {

9 - char *buf = xmalloc(path_len + 1);

9 + char *buf = xmallocz(path_len);

10 int len = 0;

12 while (len < path_len) {

grep.c

+1 -2

@@ -1741,7 +1741,7 @@ static int grep_source_load_file(struct grep_source *gs)

1741 i = open(filename, O_RDONLY);

1742 if (i < 0)

1743 goto err_ret;

1744 - data = xmalloc(size + 1);

1744 + data = xmallocz(size);

1745 if (st.st_size != read_in_full(i, data, size)) {

1746 error(_("'%s': short read %s"), filename, strerror(errno));

1747 close(i);

@@ -1749,7 +1749,6 @@ static int grep_source_load_file(struct grep_source *gs)

1749 return -1;

1750 }

1751 close(i);

1752 - data[size] = 0;

1752

1753 gs->buf = data;

1754 gs->size = size;

imap-send.c

+2 -3

@@ -892,12 +892,11 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)

892 response = xstrfmt("%s %s", user, hex);

893 resp_len = strlen(response) + 1;

894

895 - response_64 = xmalloc(ENCODED_SIZE(resp_len) + 1);

895 + response_64 = xmallocz(ENCODED_SIZE(resp_len));

896 encoded_len = EVP_EncodeBlock((unsigned char *)response_64,

897 (unsigned char *)response, resp_len);

898 if (encoded_len < 0)

899 die("EVP_EncodeBlock error");

900 - response_64[encoded_len] = '\0';

900 return (char *)response_64;

901 }

902

@@ -1188,7 +1187,7 @@ static void lf_to_crlf(struct strbuf *msg)

1187 j++;

1188 }

1189

1191 - new = xmalloc(j + 1);

1190 + new = xmallocz(j);

1191

1192 /*

1193 * Second pass: write the new string. Note that this loop is

ll-merge.c

+1 -1

@@ -205,7 +205,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,

205 if (fstat(fd, &st))

206 goto close_bad;

207 result->size = st.st_size;

208 - result->ptr = xmalloc(result->size + 1);

208 + result->ptr = xmallocz(result->size);

209 if (read_in_full(fd, result->ptr, result->size) != result->size) {

210 free(result->ptr);

211 result->ptr = NULL;

progress.c

+1 -1

@@ -247,7 +247,7 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)

247 size_t len = strlen(msg) + 5;

248 struct throughput *tp = progress->throughput;

249

250 - bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);

250 + bufp = (len < sizeof(buf)) ? buf : xmallocz(len);

251 if (tp) {

252 unsigned int rate = !tp->avg_misecs ? 0 :

253 tp->avg_bytes / tp->avg_misecs;

refs.c

+1 -1

@@ -124,7 +124,7 @@ int refname_is_safe(const char *refname)

124 char *buf;

125 int result;

126

127 - buf = xmalloc(strlen(refname) + 1);

127 + buf = xmallocz(strlen(refname));

128 /*

129 * Does the refname try to escape refs/?

130 * For example: refs/foo/../bar is safe but refs/foo/../../bar

setup.c

+2 -3

@@ -88,7 +88,7 @@ char *prefix_path_gently(const char *prefix, int len,

88 const char *orig = path;

89 char *sanitized;

90 if (is_absolute_path(orig)) {

91 - sanitized = xmalloc(strlen(path) + 1);

91 + sanitized = xmallocz(strlen(path));

92 if (remaining_prefix)

93 *remaining_prefix = 0;

94 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)

499 error_code = READ_GITFILE_ERR_OPEN_FAILED;

500 goto cleanup_return;

501 }

502 - buf = xmalloc(st.st_size + 1);

502 + buf = xmallocz(st.st_size);

503 len = read_in_full(fd, buf, st.st_size);

504 close(fd);

505 if (len != st.st_size) {

506 error_code = READ_GITFILE_ERR_READ_FAILED;

507 goto cleanup_return;

508 }

509 - buf[len] = '\0';

509 if (!starts_with(buf, "gitdir: ")) {

510 error_code = READ_GITFILE_ERR_INVALID_FORMAT;

511 goto cleanup_return;

strbuf.c

+1 -1

@@ -685,7 +685,7 @@ char *xstrdup_tolower(const char *string)

685 size_t len, i;

686

687 len = strlen(string);

688 - result = xmalloc(len + 1);

688 + result = xmallocz(len);

689 for (i = 0; i < len; i++)

690 result[i] = tolower(string[i]);

691 result[i] = '\0';