From b32fa95fd8293ebfecb2b7b6c8d460579318f9fe Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:44:25 -0500 Subject: convert trivial cases to ALLOC_ARRAY Each of these cases can be converted to use ALLOC_ARRAY or REALLOC_ARRAY, which has two advantages: 1. It automatically checks the array-size multiplication for overflow. 2. It always uses sizeof(*array) for the element-size, so that it can never go out of sync with the declared type of the array. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- combine-diff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'combine-diff.c') diff --git a/combine-diff.c b/combine-diff.c index 55713049a..a698016db 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -1372,7 +1372,7 @@ static struct combine_diff_path *find_paths_multitree( struct combine_diff_path paths_head; struct strbuf base; - parents_sha1 = xmalloc(nparent * sizeof(parents_sha1[0])); + ALLOC_ARRAY(parents_sha1, nparent); for (i = 0; i < nparent; i++) parents_sha1[i] = parents->sha1[i]; @@ -1483,7 +1483,7 @@ void diff_tree_combined(const unsigned char *sha1, if (opt->orderfile && num_paths) { struct obj_order *o; - o = xmalloc(sizeof(*o) * num_paths); + ALLOC_ARRAY(o, num_paths); for (i = 0, p = paths; p; p = p->next, i++) o[i].obj = p; order_objects(opt->orderfile, path_path, o, num_paths); -- cgit v1.2.1 From 3733e6946465d4a3a1d89026a5ec911d3af339ab Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:44:28 -0500 Subject: 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 Signed-off-by: Junio C Hamano --- combine-diff.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'combine-diff.c') diff --git a/combine-diff.c b/combine-diff.c index a698016db..890c4157b 100644 --- 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; -- cgit v1.2.1 From 96ffc06f72f693d80f05059a1f0e5ca9007d5f1b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:44:32 -0500 Subject: convert trivial cases to FLEX_ARRAY macros Using FLEX_ARRAY macros reduces the amount of manual computation size we have to do. It also ensures we don't overflow size_t, and it makes sure we write the same number of bytes that we allocated. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- combine-diff.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'combine-diff.c') diff --git a/combine-diff.c b/combine-diff.c index 890c4157b..be09a2b25 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -319,7 +319,7 @@ static void append_lost(struct sline *sline, int n, const char *line, int len) if (line[len-1] == '\n') len--; - lline = xmalloc(sizeof(*lline) + len + 1); + FLEX_ALLOC_MEM(lline, line, line, len); lline->len = len; lline->next = NULL; lline->prev = sline->plost.lost_tail; @@ -330,8 +330,6 @@ static void append_lost(struct sline *sline, int n, const char *line, int len) sline->plost.lost_tail = lline; sline->plost.len++; lline->parent_map = this_mask; - memcpy(lline->line, line, len); - lline->line[len] = 0; } struct combine_diff_state { -- cgit v1.2.1 From 50a6c8efa2bbeddf46ca34c7765024108202e04b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:44:35 -0500 Subject: use st_add and st_mult for allocation size computation If our size computation overflows size_t, we may allocate a much smaller buffer than we expected and overflow it. It's probably impossible to trigger an overflow in most of these sites in practice, but it is easy enough convert their additions and multiplications into overflow-checking variants. This may be fixing real bugs, and it makes auditing the code easier. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- combine-diff.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'combine-diff.c') diff --git a/combine-diff.c b/combine-diff.c index be09a2b25..0e1d4b089 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -189,11 +189,11 @@ static struct lline *coalesce_lines(struct lline *base, int *lenbase, * - Else if we have NEW, insert newend lline into base and * consume newend */ - lcs = xcalloc(origbaselen + 1, sizeof(int*)); - directions = xcalloc(origbaselen + 1, sizeof(enum coalesce_direction*)); + lcs = xcalloc(st_add(origbaselen, 1), sizeof(int*)); + directions = xcalloc(st_add(origbaselen, 1), sizeof(enum coalesce_direction*)); for (i = 0; i < origbaselen + 1; i++) { - lcs[i] = xcalloc(lennew + 1, sizeof(int)); - directions[i] = xcalloc(lennew + 1, sizeof(enum coalesce_direction)); + lcs[i] = xcalloc(st_add(lennew, 1), sizeof(int)); + directions[i] = xcalloc(st_add(lennew, 1), sizeof(enum coalesce_direction)); directions[i][0] = BASE; } for (j = 1; j < lennew + 1; j++) @@ -1111,7 +1111,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, if (result_size && result[result_size-1] != '\n') cnt++; /* incomplete line */ - sline = xcalloc(cnt+2, sizeof(*sline)); + sline = xcalloc(st_add(cnt, 2), sizeof(*sline)); sline[0].bol = result; for (lno = 0, cp = result; cp < result + result_size; cp++) { if (*cp == '\n') { @@ -1130,7 +1130,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, /* Even p_lno[cnt+1] is valid -- that is for the end line number * for deletion hunk at the end. */ - sline[0].p_lno = xcalloc((cnt+2) * num_parent, sizeof(unsigned long)); + sline[0].p_lno = xcalloc(st_mult(st_add(cnt, 2), num_parent), sizeof(unsigned long)); for (lno = 0; lno <= cnt; lno++) sline[lno+1].p_lno = sline[lno].p_lno + num_parent; @@ -1262,7 +1262,7 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p, struct diff_filespec *pool; pair = xmalloc(sizeof(*pair)); - pool = xcalloc(num_parent + 1, sizeof(struct diff_filespec)); + pool = xcalloc(st_add(num_parent, 1), sizeof(struct diff_filespec)); pair->one = pool + 1; pair->two = pool; -- cgit v1.2.1