From c3a700fba1693f68643f24c4f2e6d4cdd240babf Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Feb 2016 06:21:08 -0500 Subject: reflog_expire_cfg: NUL-terminate pattern field You can tweak the reflog expiration for a particular subset of refs by configuring gc.foo.reflogexpire. We keep a linked list of reflog_expire_cfg structs, each of which holds the pattern and a "len" field for the length of the pattern. The pattern itself is _not_ NUL-terminated. However, we feed the pattern directly to wildmatch(), which expects a NUL-terminated string, meaning it may keep reading random junk after our struct. We can fix this by allocating an extra byte for the NUL (which is already zero because we use xcalloc). Let's also drop the misleading "len" field, which is no longer necessary. The existing use of "len" can be converted to use strncmp(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/reflog.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'builtin') diff --git a/builtin/reflog.c b/builtin/reflog.c index f39960e5e4..9980731ee7 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -396,7 +396,6 @@ static struct reflog_expire_cfg { struct reflog_expire_cfg *next; unsigned long expire_total; unsigned long expire_unreachable; - size_t len; char pattern[FLEX_ARRAY]; } *reflog_expire_cfg, **reflog_expire_cfg_tail; @@ -408,13 +407,12 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len) reflog_expire_cfg_tail = &reflog_expire_cfg; for (ent = reflog_expire_cfg; ent; ent = ent->next) - if (ent->len == len && - !memcmp(ent->pattern, pattern, len)) + if (!strncmp(ent->pattern, pattern, len) && + ent->pattern[len] == '\0') return ent; - ent = xcalloc(1, (sizeof(*ent) + len)); + ent = xcalloc(1, sizeof(*ent) + len + 1); memcpy(ent->pattern, pattern, len); - ent->len = len; *reflog_expire_cfg_tail = ent; reflog_expire_cfg_tail = &(ent->next); return ent; -- cgit v1.3-5-g9baa From 850d2fec53ee188bab9e458f77906041ac7f1904 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:44:21 -0500 Subject: convert manual allocations to argv_array There are many manual argv allocations that predate the argv_array API. Switching to that API brings a few advantages: 1. We no longer have to manually compute the correct final array size (so it's one less thing we can screw up). 2. In many cases we had to make a separate pass to count, then allocate, then fill in the array. Now we can do it in one pass, making the code shorter and easier to follow. 3. argv_array handles memory ownership for us, making it more obvious when things should be free()d and and when not. Most of these cases are pretty straightforward. In some, we switch from "run_command_v" to "run_command" which lets us directly use the argv_array embedded in "struct child_process". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/grep.c | 10 +++++----- builtin/receive-pack.c | 12 +++--------- builtin/remote-ext.c | 26 +++++--------------------- daemon.c | 12 +++++------- git.c | 14 +++++--------- line-log.c | 22 +++++++++------------- remote-curl.c | 23 +++++++++++------------ 7 files changed, 43 insertions(+), 76 deletions(-) (limited to 'builtin') diff --git a/builtin/grep.c b/builtin/grep.c index 4229cae390..95ddf96d1e 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -354,17 +354,17 @@ static void append_path(struct grep_opt *opt, const void *data, size_t len) static void run_pager(struct grep_opt *opt, const char *prefix) { struct string_list *path_list = opt->output_priv; - const char **argv = xmalloc(sizeof(const char *) * (path_list->nr + 1)); + struct child_process child = CHILD_PROCESS_INIT; int i, status; for (i = 0; i < path_list->nr; i++) - argv[i] = path_list->items[i].string; - argv[path_list->nr] = NULL; + argv_array_push(&child.args, path_list->items[i].string); + child.dir = prefix; + child.use_shell = 1; - status = run_command_v_opt_cd_env(argv, RUN_USING_SHELL, prefix, NULL); + status = run_command(&child); if (status) exit(status); - free(argv); } static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int cached) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index f2d6761af6..932afab931 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1031,7 +1031,6 @@ static void run_update_post_hook(struct command *commands) { struct command *cmd; int argc; - const char **argv; struct child_process proc = CHILD_PROCESS_INIT; const char *hook; @@ -1044,21 +1043,16 @@ static void run_update_post_hook(struct command *commands) if (!argc || !hook) return; - argv = xmalloc(sizeof(*argv) * (2 + argc)); - argv[0] = hook; - - for (argc = 1, cmd = commands; cmd; cmd = cmd->next) { + argv_array_push(&proc.args, hook); + for (cmd = commands; cmd; cmd = cmd->next) { if (cmd->error_string || cmd->did_not_exist) continue; - argv[argc] = xstrdup(cmd->ref_name); - argc++; + argv_array_push(&proc.args, cmd->ref_name); } - argv[argc] = NULL; proc.no_stdin = 1; proc.stdout_to_stderr = 1; proc.err = use_sideband ? -1 : 0; - proc.argv = argv; if (!start_command(&proc)) { if (use_sideband) diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c index e3cd25d580..7457c743e8 100644 --- a/builtin/remote-ext.c +++ b/builtin/remote-ext.c @@ -114,30 +114,14 @@ static char *strip_escapes(const char *str, const char *service, } } -/* Should be enough... */ -#define MAXARGUMENTS 256 - -static const char **parse_argv(const char *arg, const char *service) +static void parse_argv(struct argv_array *out, const char *arg, const char *service) { - int arguments = 0; - int i; - const char **ret; - char *temparray[MAXARGUMENTS + 1]; - while (*arg) { - char *expanded; - if (arguments == MAXARGUMENTS) - die("remote-ext command has too many arguments"); - expanded = strip_escapes(arg, service, &arg); + char *expanded = strip_escapes(arg, service, &arg); if (expanded) - temparray[arguments++] = expanded; + argv_array_push(out, expanded); + free(expanded); } - - ret = xmalloc((arguments + 1) * sizeof(char *)); - for (i = 0; i < arguments; i++) - ret[i] = temparray[i]; - ret[arguments] = NULL; - return ret; } static void send_git_request(int stdin_fd, const char *serv, const char *repo, @@ -158,7 +142,7 @@ static int run_child(const char *arg, const char *service) child.in = -1; child.out = -1; child.err = 0; - child.argv = parse_argv(arg, service); + parse_argv(&child.args, arg, service); if (start_command(&child) < 0) die("Can't run specified command"); diff --git a/daemon.c b/daemon.c index be70cd4da0..1e258ac8f9 100644 --- a/daemon.c +++ b/daemon.c @@ -808,7 +808,7 @@ static void check_dead_children(void) cradle = &blanket->next; } -static char **cld_argv; +static struct argv_array cld_argv = ARGV_ARRAY_INIT; static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen) { struct child_process cld = CHILD_PROCESS_INIT; @@ -842,7 +842,7 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen) #endif } - cld.argv = (const char **)cld_argv; + cld.argv = cld_argv.argv; cld.in = incoming; cld.out = dup(incoming); @@ -1374,12 +1374,10 @@ int main(int argc, char **argv) write_file(pid_file, "%"PRIuMAX, (uintmax_t) getpid()); /* prepare argv for serving-processes */ - cld_argv = xmalloc(sizeof (char *) * (argc + 2)); - cld_argv[0] = argv[0]; /* git-daemon */ - cld_argv[1] = "--serve"; + argv_array_push(&cld_argv, argv[0]); /* git-daemon */ + argv_array_push(&cld_argv, "--serve"); for (i = 1; i < argc; ++i) - cld_argv[i+1] = argv[i]; - cld_argv[argc+1] = NULL; + argv_array_push(&cld_argv, argv[i]); return serve(&listen_addr, listen_port, cred); } diff --git a/git.c b/git.c index 6ed824cacf..e61a59c014 100644 --- a/git.c +++ b/git.c @@ -239,19 +239,15 @@ static int handle_alias(int *argcp, const char ***argv) alias_string = alias_lookup(alias_command); if (alias_string) { if (alias_string[0] == '!') { - const char **alias_argv; - int argc = *argcp, i; + struct child_process child = CHILD_PROCESS_INIT; commit_pager_choice(); - /* build alias_argv */ - alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 1)); - alias_argv[0] = alias_string + 1; - for (i = 1; i < argc; ++i) - alias_argv[i] = (*argv)[i]; - alias_argv[argc] = NULL; + child.use_shell = 1; + argv_array_push(&child.args, alias_string + 1); + argv_array_pushv(&child.args, (*argv) + 1); - ret = run_command_v_opt(alias_argv, RUN_USING_SHELL); + ret = run_command(&child); if (ret >= 0) /* normal exit */ exit(ret); diff --git a/line-log.c b/line-log.c index af6e2f799e..5877986c5a 100644 --- a/line-log.c +++ b/line-log.c @@ -14,6 +14,7 @@ #include "graph.h" #include "userdiff.h" #include "line-log.h" +#include "argv-array.h" static void range_set_grow(struct range_set *rs, size_t extra) { @@ -746,22 +747,17 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list add_line_range(rev, commit, range); if (!rev->diffopt.detect_rename) { - int i, count = 0; - struct line_log_data *r = range; + struct line_log_data *r; + struct argv_array array = ARGV_ARRAY_INIT; const char **paths; - while (r) { - count++; - r = r->next; - } - paths = xmalloc((count+1)*sizeof(char *)); - r = range; - for (i = 0; i < count; i++) { - paths[i] = xstrdup(r->path); - r = r->next; - } - paths[count] = NULL; + + for (r = range; r; r = r->next) + argv_array_push(&array, r->path); + paths = argv_array_detach(&array); + parse_pathspec(&rev->diffopt.pathspec, 0, PATHSPEC_PREFER_FULL, "", paths); + /* strings are now owned by pathspec */ free(paths); } } diff --git a/remote-curl.c b/remote-curl.c index f404faf0f4..e81b9d560b 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -845,23 +845,22 @@ static void parse_fetch(struct strbuf *buf) static int push_dav(int nr_spec, char **specs) { - const char **argv = xmalloc((10 + nr_spec) * sizeof(char*)); - int argc = 0, i; + struct child_process child = CHILD_PROCESS_INIT; + size_t i; - argv[argc++] = "http-push"; - argv[argc++] = "--helper-status"; + child.git_cmd = 1; + argv_array_push(&child.args, "http-push"); + argv_array_push(&child.args, "--helper-status"); if (options.dry_run) - argv[argc++] = "--dry-run"; + argv_array_push(&child.args, "--dry-run"); if (options.verbosity > 1) - argv[argc++] = "--verbose"; - argv[argc++] = url.buf; + argv_array_push(&child.args, "--verbose"); + argv_array_push(&child.args, url.buf); for (i = 0; i < nr_spec; i++) - argv[argc++] = specs[i]; - argv[argc++] = NULL; + argv_array_push(&child.args, specs[i]); - if (run_command_v_opt(argv, RUN_GIT_CMD)) - die("git-%s failed", argv[0]); - free(argv); + if (run_command(&child)) + die("git-http-push failed"); return 0; } -- cgit v1.3-5-g9baa 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 --- alias.c | 2 +- attr.c | 2 +- bisect.c | 4 ++-- builtin/blame.c | 3 ++- builtin/clean.c | 2 +- builtin/fast-export.c | 2 +- builtin/index-pack.c | 4 ++-- builtin/merge-base.c | 2 +- builtin/mv.c | 3 ++- builtin/pack-objects.c | 7 ++++--- builtin/pack-redundant.c | 2 +- builtin/receive-pack.c | 5 ++--- column.c | 2 +- combine-diff.c | 4 ++-- commit.c | 2 +- compat/mingw.c | 6 +++--- diffcore-order.c | 4 ++-- dir.c | 6 +++--- fast-import.c | 5 +++-- fsck.c | 3 ++- graph.c | 10 ++++------ khash.h | 2 +- levenshtein.c | 8 +++++--- line-log.c | 8 ++++---- notes.c | 2 +- pack-check.c | 2 +- pack-revindex.c | 12 ++++++++---- pathspec.c | 5 +++-- remote-curl.c | 3 ++- sha1_file.c | 4 ++-- shallow.c | 6 +++--- show-index.c | 3 ++- transport.c | 2 +- xdiff-interface.c | 2 +- 34 files changed, 75 insertions(+), 64 deletions(-) (limited to 'builtin') diff --git a/alias.c b/alias.c index a11229db9e..3b90397a99 100644 --- a/alias.c +++ b/alias.c @@ -23,7 +23,7 @@ int split_cmdline(char *cmdline, const char ***argv) int src, dst, count = 0, size = 16; char quoted = 0; - *argv = xmalloc(sizeof(**argv) * size); + ALLOC_ARRAY(*argv, size); /* split alias_string */ (*argv)[count++] = cmdline; diff --git a/attr.c b/attr.c index 086c08dcfa..c83ec49f18 100644 --- a/attr.c +++ b/attr.c @@ -799,7 +799,7 @@ int git_all_attrs(const char *path, int *num, struct git_attr_check **check) ++count; } *num = count; - *check = xmalloc(sizeof(**check) * count); + ALLOC_ARRAY(*check, count); j = 0; for (i = 0; i < attr_nr; i++) { const char *value = check_all_attr[i].value; diff --git a/bisect.c b/bisect.c index 42aa7aa6aa..6d0793b9fc 100644 --- a/bisect.c +++ b/bisect.c @@ -708,10 +708,10 @@ static struct commit *get_commit_reference(const unsigned char *sha1) static struct commit **get_bad_and_good_commits(int *rev_nr) { - int len = 1 + good_revs.nr; - struct commit **rev = xmalloc(len * sizeof(*rev)); + struct commit **rev; int i, n = 0; + ALLOC_ARRAY(rev, 1 + good_revs.nr); rev[n++] = get_commit_reference(current_bad_oid->hash); for (i = 0; i < good_revs.nr; i++) rev[n++] = get_commit_reference(good_revs.sha1[i]); diff --git a/builtin/blame.c b/builtin/blame.c index 1df13cf7f4..4de9e10148 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2042,7 +2042,8 @@ static int prepare_lines(struct scoreboard *sb) for (p = buf; p < end; p = get_next_line(p, end)) num++; - sb->lineno = lineno = xmalloc(sizeof(*sb->lineno) * (num + 1)); + ALLOC_ARRAY(sb->lineno, num + 1); + lineno = sb->lineno; for (p = buf; p < end; p = get_next_line(p, end)) *lineno++ = p - buf; diff --git a/builtin/clean.c b/builtin/clean.c index 919157bc2f..1f02c8294c 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -543,7 +543,7 @@ static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff) int eof = 0; int i; - chosen = xmalloc(sizeof(int) * stuff->nr); + ALLOC_ARRAY(chosen, stuff->nr); /* set chosen as uninitialized */ for (i = 0; i < stuff->nr; i++) chosen[i] = -1; diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 2471297f71..8164b581a6 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -1021,7 +1021,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix) const char **refspecs_str; int i; - refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr); + ALLOC_ARRAY(refspecs_str, refspecs_list.nr); for (i = 0; i < refspecs_list.nr; i++) refspecs_str[i] = refspecs_list.items[i].string; diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 6a01509587..a60bcfac06 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1346,7 +1346,7 @@ static void fix_unresolved_deltas(struct sha1file *f) * before deltas depending on them, a good heuristic is to start * resolving deltas in the same order as their position in the pack. */ - sorted_by_pos = xmalloc(nr_ref_deltas * sizeof(*sorted_by_pos)); + ALLOC_ARRAY(sorted_by_pos, nr_ref_deltas); for (i = 0; i < nr_ref_deltas; i++) sorted_by_pos[i] = &ref_deltas[i]; qsort(sorted_by_pos, nr_ref_deltas, sizeof(*sorted_by_pos), delta_pos_compare); @@ -1759,7 +1759,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) if (show_stat) show_pack_info(stat_only); - idx_objects = xmalloc((nr_objects) * sizeof(struct pack_idx_entry *)); + ALLOC_ARRAY(idx_objects, nr_objects); for (i = 0; i < nr_objects; i++) idx_objects[i] = &objects[i].idx; curr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_sha1); diff --git a/builtin/merge-base.c b/builtin/merge-base.c index a8911626c2..c0d1822eb3 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -252,7 +252,7 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix) if (argc < 2) usage_with_options(merge_base_usage, options); - rev = xmalloc(argc * sizeof(*rev)); + ALLOC_ARRAY(rev, argc); while (argc-- > 0) rev[rev_nr++] = get_commit_reference(*argv++); return show_merge_base(rev, rev_nr, show_all); diff --git a/builtin/mv.c b/builtin/mv.c index d1d43168ae..9a9813a0ec 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -24,7 +24,8 @@ static const char **internal_copy_pathspec(const char *prefix, int count, unsigned flags) { int i; - const char **result = xmalloc((count + 1) * sizeof(const char *)); + const char **result; + ALLOC_ARRAY(result, count + 1); memcpy(result, pathspec, count * sizeof(const char *)); result[count] = NULL; for (i = 0; i < count; i++) { diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 4dae5b11c2..b4f1fa6d33 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -624,7 +624,7 @@ static struct object_entry **compute_write_order(void) { unsigned int i, wo_end, last_untagged; - struct object_entry **wo = xmalloc(to_pack.nr_objects * sizeof(*wo)); + struct object_entry **wo; struct object_entry *objects = to_pack.objects; for (i = 0; i < to_pack.nr_objects; i++) { @@ -657,6 +657,7 @@ static struct object_entry **compute_write_order(void) * Give the objects in the original recency order until * we see a tagged tip. */ + ALLOC_ARRAY(wo, to_pack.nr_objects); for (i = wo_end = 0; i < to_pack.nr_objects; i++) { if (objects[i].tagged) break; @@ -769,7 +770,7 @@ static void write_pack_file(void) if (progress > pack_to_stdout) progress_state = start_progress(_("Writing objects"), nr_result); - written_list = xmalloc(to_pack.nr_objects * sizeof(*written_list)); + ALLOC_ARRAY(written_list, to_pack.nr_objects); write_order = compute_write_order(); do { @@ -2129,7 +2130,7 @@ static void prepare_pack(int window, int depth) if (!to_pack.nr_objects || !window || !depth) return; - delta_list = xmalloc(to_pack.nr_objects * sizeof(*delta_list)); + ALLOC_ARRAY(delta_list, to_pack.nr_objects); nr_deltas = n = 0; for (i = 0; i < to_pack.nr_objects; i++) { diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index d0532f66b1..72c815844d 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c @@ -53,7 +53,7 @@ static inline struct llist_item *llist_item_get(void) free_nodes = free_nodes->next; } else { int i = 1; - new = xmalloc(sizeof(struct llist_item) * BLKSIZE); + ALLOC_ARRAY(new, BLKSIZE); for (; i < BLKSIZE; i++) llist_item_put(&new[i]); } diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 932afab931..3dc3868c86 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1591,8 +1591,7 @@ static void prepare_shallow_update(struct command *commands, { int i, j, k, bitmap_size = (si->ref->nr + 31) / 32; - si->used_shallow = xmalloc(sizeof(*si->used_shallow) * - si->shallow->nr); + ALLOC_ARRAY(si->used_shallow, si->shallow->nr); assign_shallow_commits_to_refs(si, si->used_shallow, NULL); si->need_reachability_test = @@ -1658,7 +1657,7 @@ static void update_shallow_info(struct command *commands, return; } - ref_status = xmalloc(sizeof(*ref_status) * ref->nr); + ALLOC_ARRAY(ref_status, ref->nr); assign_shallow_commits_to_refs(si, NULL, ref_status); for (cmd = commands; cmd; cmd = cmd->next) { if (is_null_sha1(cmd->new_sha1)) diff --git a/column.c b/column.c index 786abe62b0..f9fda681f1 100644 --- a/column.c +++ b/column.c @@ -164,7 +164,7 @@ static void display_table(const struct string_list *list, data.colopts = colopts; data.opts = *opts; - data.len = xmalloc(sizeof(*data.len) * list->nr); + ALLOC_ARRAY(data.len, list->nr); for (i = 0; i < list->nr; i++) data.len[i] = item_length(colopts, list->items[i].string); diff --git a/combine-diff.c b/combine-diff.c index 55713049a4..a698016db7 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); diff --git a/commit.c b/commit.c index 40388d71e7..31cd91f81e 100644 --- a/commit.c +++ b/commit.c @@ -903,7 +903,7 @@ static int remove_redundant(struct commit **array, int cnt) work = xcalloc(cnt, sizeof(*work)); redundant = xcalloc(cnt, 1); - filled_index = xmalloc(sizeof(*filled_index) * (cnt - 1)); + ALLOC_ARRAY(filled_index, cnt - 1); for (i = 0; i < cnt; i++) parse_commit(array[i]); diff --git a/compat/mingw.c b/compat/mingw.c index 9b2a1f552e..d8a5345a30 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -852,7 +852,7 @@ static char **get_path_split(void) if (!n) return NULL; - path = xmalloc((n+1)*sizeof(char *)); + ALLOC_ARRAY(path, n + 1); p = envpath; i = 0; do { @@ -937,7 +937,7 @@ static wchar_t *make_environment_block(char **deltaenv) i++; /* copy the environment, leaving space for changes */ - tmpenv = xmalloc((size + i) * sizeof(char*)); + ALLOC_ARRAY(tmpenv, size + i); memcpy(tmpenv, environ, size * sizeof(char*)); /* merge supplied environment changes into the temporary environment */ @@ -1127,7 +1127,7 @@ static int try_shell_exec(const char *cmd, char *const *argv) int argc = 0; const char **argv2; while (argv[argc]) argc++; - argv2 = xmalloc(sizeof(*argv) * (argc+1)); + ALLOC_ARRAY(argv2, argc + 1); argv2[0] = (char *)cmd; /* full path to the script file */ memcpy(&argv2[1], &argv[1], sizeof(*argv) * argc); pid = mingw_spawnv(prog, argv2, 1); diff --git a/diffcore-order.c b/diffcore-order.c index 97dd3d0095..69d41f7a57 100644 --- a/diffcore-order.c +++ b/diffcore-order.c @@ -52,7 +52,7 @@ static void prepare_order(const char *orderfile) } if (pass == 0) { order_cnt = cnt; - order = xmalloc(sizeof(*order) * cnt); + ALLOC_ARRAY(order, cnt); } } } @@ -120,7 +120,7 @@ void diffcore_order(const char *orderfile) if (!q->nr) return; - o = xmalloc(sizeof(*o) * q->nr); + ALLOC_ARRAY(o, q->nr); for (i = 0; i < q->nr; i++) o[i].obj = q->queue[i]; order_objects(orderfile, pair_pathtwo, o, q->nr); diff --git a/dir.c b/dir.c index 29aec12487..363a402887 100644 --- a/dir.c +++ b/dir.c @@ -2448,14 +2448,14 @@ static int read_one_dir(struct untracked_cache_dir **untracked_, ud.untracked_alloc = value; ud.untracked_nr = value; if (ud.untracked_nr) - ud.untracked = xmalloc(sizeof(*ud.untracked) * ud.untracked_nr); + ALLOC_ARRAY(ud.untracked, ud.untracked_nr); data = next; next = data; ud.dirs_alloc = ud.dirs_nr = decode_varint(&next); if (next > end) return -1; - ud.dirs = xmalloc(sizeof(*ud.dirs) * ud.dirs_nr); + ALLOC_ARRAY(ud.dirs, ud.dirs_nr); data = next; len = strlen((const char *)data); @@ -2575,7 +2575,7 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long rd.data = next; rd.end = end; rd.index = 0; - rd.ucd = xmalloc(sizeof(*rd.ucd) * len); + ALLOC_ARRAY(rd.ucd, len); if (read_one_dir(&uc->root, &rd) || rd.index != len) goto done; diff --git a/fast-import.c b/fast-import.c index 3c65edb5c4..36dbaa5b16 100644 --- a/fast-import.c +++ b/fast-import.c @@ -814,7 +814,8 @@ static struct tree_entry *new_tree_entry(void) if (!avail_tree_entry) { unsigned int n = tree_entry_alloc; total_allocd += n * sizeof(struct tree_entry); - avail_tree_entry = e = xmalloc(n * sizeof(struct tree_entry)); + ALLOC_ARRAY(e, n); + avail_tree_entry = e; while (n-- > 1) { *((void**)e) = e + 1; e++; @@ -898,7 +899,7 @@ static const char *create_index(void) struct object_entry_pool *o; /* Build the table of object IDs. */ - idx = xmalloc(object_count * sizeof(*idx)); + ALLOC_ARRAY(idx, object_count); c = idx; for (o = blocks; o; o = o->next_pool) for (e = o->next_free; e-- != o->entries;) diff --git a/fsck.c b/fsck.c index c637f6676b..ca4c685377 100644 --- a/fsck.c +++ b/fsck.c @@ -199,7 +199,8 @@ void fsck_set_msg_type(struct fsck_options *options, if (!options->msg_type) { int i; - int *msg_type = xmalloc(sizeof(int) * FSCK_MSG_MAX); + int *msg_type; + ALLOC_ARRAY(msg_type, FSCK_MSG_MAX); for (i = 0; i < FSCK_MSG_MAX; i++) msg_type[i] = fsck_msg_type(i, options); options->msg_type = msg_type; diff --git a/graph.c b/graph.c index c25a09a8fd..1350bdde3b 100644 --- a/graph.c +++ b/graph.c @@ -234,12 +234,10 @@ struct git_graph *graph_init(struct rev_info *opt) * We'll automatically grow columns later if we need more room. */ graph->column_capacity = 30; - graph->columns = xmalloc(sizeof(struct column) * - graph->column_capacity); - graph->new_columns = xmalloc(sizeof(struct column) * - graph->column_capacity); - graph->mapping = xmalloc(sizeof(int) * 2 * graph->column_capacity); - graph->new_mapping = xmalloc(sizeof(int) * 2 * graph->column_capacity); + ALLOC_ARRAY(graph->columns, graph->column_capacity); + ALLOC_ARRAY(graph->new_columns, graph->column_capacity); + ALLOC_ARRAY(graph->mapping, 2 * graph->column_capacity); + ALLOC_ARRAY(graph->new_mapping, 2 * graph->column_capacity); /* * The diff output prefix callback, with this we can make diff --git a/khash.h b/khash.h index 376475a5ea..c0da40daa7 100644 --- a/khash.h +++ b/khash.h @@ -117,7 +117,7 @@ static const double __ac_HASH_UPPER = 0.77; if (new_n_buckets < 4) new_n_buckets = 4; \ if (h->size >= (khint_t)(new_n_buckets * __ac_HASH_UPPER + 0.5)) j = 0; /* requested size is too small */ \ else { /* hash table size to be changed (shrink or expand); rehash */ \ - new_flags = (khint32_t*)xmalloc(__ac_fsize(new_n_buckets) * sizeof(khint32_t)); \ + ALLOC_ARRAY(new_flags, __ac_fsize(new_n_buckets)); \ if (!new_flags) return -1; \ memset(new_flags, 0xaa, __ac_fsize(new_n_buckets) * sizeof(khint32_t)); \ if (h->n_buckets < new_n_buckets) { /* expand */ \ diff --git a/levenshtein.c b/levenshtein.c index fc281597fd..d2632690d5 100644 --- a/levenshtein.c +++ b/levenshtein.c @@ -42,11 +42,13 @@ int levenshtein(const char *string1, const char *string2, int w, int s, int a, int d) { int len1 = strlen(string1), len2 = strlen(string2); - int *row0 = xmalloc(sizeof(int) * (len2 + 1)); - int *row1 = xmalloc(sizeof(int) * (len2 + 1)); - int *row2 = xmalloc(sizeof(int) * (len2 + 1)); + int *row0, *row1, *row2; int i, j; + ALLOC_ARRAY(row0, len2 + 1); + ALLOC_ARRAY(row1, len2 + 1); + ALLOC_ARRAY(row2, len2 + 1); + for (j = 0; j <= len2; j++) row1[j] = j * a; for (i = 0; i < len1; i++) { diff --git a/line-log.c b/line-log.c index 5877986c5a..bbe31ed6fb 100644 --- a/line-log.c +++ b/line-log.c @@ -522,7 +522,7 @@ static void fill_line_ends(struct diff_filespec *spec, long *lines, if (diff_populate_filespec(spec, 0)) die("Cannot read blob %s", sha1_to_hex(spec->sha1)); - ends = xmalloc(size * sizeof(*ends)); + ALLOC_ARRAY(ends, size); ends[cur++] = 0; data = spec->data; while (num < spec->size) { @@ -1142,9 +1142,9 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm if (nparents > 1 && rev->first_parent_only) nparents = 1; - diffqueues = xmalloc(nparents * sizeof(*diffqueues)); - cand = xmalloc(nparents * sizeof(*cand)); - parents = xmalloc(nparents * sizeof(*parents)); + ALLOC_ARRAY(diffqueues, nparents); + ALLOC_ARRAY(cand, nparents); + ALLOC_ARRAY(parents, nparents); p = commit->parents; for (i = 0; i < nparents; i++) { diff --git a/notes.c b/notes.c index db77922130..8b59ccdb11 100644 --- a/notes.c +++ b/notes.c @@ -1032,7 +1032,7 @@ struct notes_tree **load_notes_trees(struct string_list *refs) struct string_list_item *item; int counter = 0; struct notes_tree **trees; - trees = xmalloc((refs->nr+1) * sizeof(struct notes_tree *)); + ALLOC_ARRAY(trees, refs->nr + 1); for_each_string_list_item(item, refs) { struct notes_tree *t = xcalloc(1, sizeof(struct notes_tree)); init_notes(t, item->string, combine_notes_ignore, 0); diff --git a/pack-check.c b/pack-check.c index 433bd86ccd..1da89a41ce 100644 --- a/pack-check.c +++ b/pack-check.c @@ -89,7 +89,7 @@ static int verify_packfile(struct packed_git *p, * we do not do scan-streaming check on the pack file. */ nr_objects = p->num_objects; - entries = xmalloc((nr_objects + 1) * sizeof(*entries)); + ALLOC_ARRAY(entries, nr_objects + 1); entries[nr_objects].offset = pack_sig_ofs; /* first sort entries by pack offset, since unpacking them is more efficient that way */ for (i = 0; i < nr_objects; i++) { diff --git a/pack-revindex.c b/pack-revindex.c index e542ea7703..f6a3613beb 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -83,10 +83,14 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max) * keep track of them with alias pointers, always sorting from "from" * to "to". */ - struct revindex_entry *tmp = xmalloc(n * sizeof(*tmp)); - struct revindex_entry *from = entries, *to = tmp; + struct revindex_entry *tmp, *from, *to; int bits; - unsigned *pos = xmalloc(BUCKETS * sizeof(*pos)); + unsigned *pos; + + ALLOC_ARRAY(pos, BUCKETS); + ALLOC_ARRAY(tmp, n); + from = entries; + to = tmp; /* * If (max >> bits) is zero, then we know that the radix digit we are @@ -161,7 +165,7 @@ static void create_pack_revindex(struct pack_revindex *rix) unsigned i; const char *index = p->index_data; - rix->revindex = xmalloc(sizeof(*rix->revindex) * (num_ent + 1)); + ALLOC_ARRAY(rix->revindex, num_ent + 1); index += 4 * 256; if (p->index_version > 1) { diff --git a/pathspec.c b/pathspec.c index 9304ee33d7..c9e9b6c077 100644 --- a/pathspec.c +++ b/pathspec.c @@ -406,7 +406,8 @@ void parse_pathspec(struct pathspec *pathspec, n++; pathspec->nr = n; - pathspec->items = item = xmalloc(sizeof(*item) * n); + ALLOC_ARRAY(pathspec->items, n); + item = pathspec->items; pathspec->_raw = argv; prefixlen = prefix ? strlen(prefix) : 0; @@ -483,7 +484,7 @@ const char **get_pathspec(const char *prefix, const char **pathspec) void copy_pathspec(struct pathspec *dst, const struct pathspec *src) { *dst = *src; - dst->items = xmalloc(sizeof(struct pathspec_item) * dst->nr); + ALLOC_ARRAY(dst->items, dst->nr); memcpy(dst->items, src->items, sizeof(struct pathspec_item) * dst->nr); } diff --git a/remote-curl.c b/remote-curl.c index e81b9d560b..e85333a51b 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -696,9 +696,10 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads) static int fetch_dumb(int nr_heads, struct ref **to_fetch) { struct walker *walker; - char **targets = xmalloc(nr_heads * sizeof(char*)); + char **targets; int ret, i; + ALLOC_ARRAY(targets, nr_heads); if (options.depth) die("dumb http transport does not support --depth"); for (i = 0; i < nr_heads; i++) diff --git a/sha1_file.c b/sha1_file.c index 73ccd49a46..d14abfed9b 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1942,7 +1942,7 @@ static enum object_type packed_to_object_type(struct packed_git *p, /* Push the object we're going to leave behind */ if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) { poi_stack_alloc = alloc_nr(poi_stack_nr); - poi_stack = xmalloc(sizeof(off_t)*poi_stack_alloc); + ALLOC_ARRAY(poi_stack, poi_stack_alloc); memcpy(poi_stack, small_poi_stack, sizeof(off_t)*poi_stack_nr); } else { ALLOC_GROW(poi_stack, poi_stack_nr+1, poi_stack_alloc); @@ -2308,7 +2308,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset, if (delta_stack_nr >= delta_stack_alloc && delta_stack == small_delta_stack) { delta_stack_alloc = alloc_nr(delta_stack_nr); - delta_stack = xmalloc(sizeof(*delta_stack)*delta_stack_alloc); + ALLOC_ARRAY(delta_stack, delta_stack_alloc); memcpy(delta_stack, small_delta_stack, sizeof(*delta_stack)*delta_stack_nr); } else { diff --git a/shallow.c b/shallow.c index 60f1505d97..71163bf3d7 100644 --- a/shallow.c +++ b/shallow.c @@ -315,8 +315,8 @@ void prepare_shallow_info(struct shallow_info *info, struct sha1_array *sa) info->shallow = sa; if (!sa) return; - info->ours = xmalloc(sizeof(*info->ours) * sa->nr); - info->theirs = xmalloc(sizeof(*info->theirs) * sa->nr); + ALLOC_ARRAY(info->ours, sa->nr); + ALLOC_ARRAY(info->theirs, sa->nr); for (i = 0; i < sa->nr; i++) { if (has_sha1_file(sa->sha1[i])) { struct commit_graft *graft; @@ -487,7 +487,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info, struct paint_info pi; trace_printf_key(&trace_shallow, "shallow: assign_shallow_commits_to_refs\n"); - shallow = xmalloc(sizeof(*shallow) * (info->nr_ours + info->nr_theirs)); + ALLOC_ARRAY(shallow, info->nr_ours + info->nr_theirs); for (i = 0; i < info->nr_ours; i++) shallow[nr_shallow++] = info->ours[i]; for (i = 0; i < info->nr_theirs; i++) diff --git a/show-index.c b/show-index.c index d9e4903fed..acf8d5445a 100644 --- a/show-index.c +++ b/show-index.c @@ -50,7 +50,8 @@ int main(int argc, char **argv) unsigned char sha1[20]; uint32_t crc; uint32_t off; - } *entries = xmalloc(nr * sizeof(entries[0])); + } *entries; + ALLOC_ARRAY(entries, nr); for (i = 0; i < nr; i++) if (fread(entries[i].sha1, 20, 1, stdin) != 1) die("unable to read sha1 %u/%u", i, nr); diff --git a/transport.c b/transport.c index 67f366687c..163abedcb0 100644 --- a/transport.c +++ b/transport.c @@ -1306,7 +1306,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs) * This condition shouldn't be met in a non-deepening fetch * (see builtin/fetch.c:quickfetch()). */ - heads = xmalloc(nr_refs * sizeof(*heads)); + ALLOC_ARRAY(heads, nr_refs); for (rm = refs; rm; rm = rm->next) heads[nr_heads++] = rm; } diff --git a/xdiff-interface.c b/xdiff-interface.c index cb67c1c42b..54236f24b9 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -265,7 +265,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags) for (i = 0, regs->nr = 1; value[i]; i++) if (value[i] == '\n') regs->nr++; - regs->array = xmalloc(regs->nr * sizeof(struct ff_reg)); + ALLOC_ARRAY(regs->array, regs->nr); for (i = 0; i < regs->nr; i++) { struct ff_reg *reg = regs->array + i; const char *ep = strchr(value, '\n'), *expression; -- cgit v1.3-5-g9baa 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 --- builtin/check-ref-format.c | 2 +- builtin/merge-tree.c | 2 +- builtin/worktree.c | 2 +- column.c | 3 +-- combine-diff.c | 4 +--- config.c | 4 +--- dir.c | 2 +- entry.c | 2 +- grep.c | 3 +-- imap-send.c | 5 ++--- ll-merge.c | 2 +- progress.c | 2 +- refs.c | 2 +- setup.c | 5 ++--- strbuf.c | 2 +- 15 files changed, 17 insertions(+), 25 deletions(-) (limited to 'builtin') diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c index fd915d5984..eac499450f 100644 --- 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 d4f0cbd451..ca570041df 100644 --- 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 475b9581a5..0a45710be8 100644 --- 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 f9fda681f1..d55ead18ef 100644 --- 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 a698016db7..890c4157bd 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; diff --git a/config.c b/config.c index 86a5eb2571..ba8fd13765 100644 --- 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 363a402887..cb5bff8d8f 100644 --- 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 582c40071a..a4109574fa 100644 --- 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 7b2b96a437..528b652f71 100644 --- 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 4d3b7737a9..2c52027c84 100644 --- 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 0338630fc2..ff4a43a982 100644 --- 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 353bd37416..76a88c573f 100644 --- 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 e2d34b253e..1d9e2a7932 100644 --- 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 2c4b22c845..669062a090 100644 --- 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 d76f0aed85..de7a7c2730 100644 --- 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'; -- cgit v1.3-5-g9baa 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 --- attr.c | 4 +--- builtin/blame.c | 4 +--- builtin/help.c | 9 +++------ builtin/mktree.c | 9 +++++---- builtin/reflog.c | 7 ++----- cache-tree.c | 4 +--- combine-diff.c | 4 +--- diff.c | 7 ++----- dir.c | 16 +++------------- hashmap.c | 3 +-- help.c | 6 ++---- log-tree.c | 5 ++--- name-hash.c | 3 +-- ref-filter.c | 6 ++---- refs.c | 6 ++---- refs/files-backend.c | 19 +++++-------------- remote.c | 5 +---- 17 files changed, 35 insertions(+), 82 deletions(-) (limited to 'builtin') diff --git a/attr.c b/attr.c index c83ec49f18..6537a433da 100644 --- a/attr.c +++ b/attr.c @@ -93,9 +93,7 @@ static struct git_attr *git_attr_internal(const char *name, int len) if (invalid_attr_name(name, len)) return NULL; - a = xmalloc(sizeof(*a) + len + 1); - memcpy(a->name, name, len); - a->name[len] = 0; + FLEX_ALLOC_MEM(a, name, name, len); a->h = hval; a->next = git_attr_hash[pos]; a->attr_nr = attr_nr++; diff --git a/builtin/blame.c b/builtin/blame.c index 4de9e10148..e175d86e56 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -459,13 +459,11 @@ static void queue_blames(struct scoreboard *sb, struct origin *porigin, static struct origin *make_origin(struct commit *commit, const char *path) { struct origin *o; - size_t pathlen = strlen(path) + 1; - o = xcalloc(1, sizeof(*o) + pathlen); + FLEX_ALLOC_STR(o, path, path); o->commit = commit; o->refcnt = 1; o->next = commit->util; commit->util = o; - memcpy(o->path, path, pathlen); /* includes NUL */ return o; } diff --git a/builtin/help.c b/builtin/help.c index 1cd0c1ee44..3c55ce4563 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -171,12 +171,10 @@ static void exec_man_cmd(const char *cmd, const char *page) static void add_man_viewer(const char *name) { struct man_viewer_list **p = &man_viewer_list; - size_t len = strlen(name); while (*p) p = &((*p)->next); - *p = xcalloc(1, (sizeof(**p) + len + 1)); - memcpy((*p)->name, name, len); /* NUL-terminated by xcalloc */ + FLEX_ALLOC_STR(*p, name, name); } static int supported_man_viewer(const char *name, size_t len) @@ -190,9 +188,8 @@ static void do_add_man_viewer_info(const char *name, size_t len, const char *value) { - struct man_viewer_info_list *new = xcalloc(1, sizeof(*new) + len + 1); - - memcpy(new->name, name, len); /* NUL-terminated by xcalloc */ + struct man_viewer_info_list *new; + FLEX_ALLOC_MEM(new, name, name, len); new->info = xstrdup(value); new->next = man_viewer_info_list; man_viewer_info_list = new; diff --git a/builtin/mktree.c b/builtin/mktree.c index a964d6be52..b0aab65353 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -19,16 +19,17 @@ static int alloc, used; static void append_to_tree(unsigned mode, unsigned char *sha1, char *path) { struct treeent *ent; - int len = strlen(path); + size_t len = strlen(path); if (strchr(path, '/')) die("path %s contains slash", path); - ALLOC_GROW(entries, used + 1, alloc); - ent = entries[used++] = xmalloc(sizeof(**entries) + len + 1); + FLEX_ALLOC_MEM(ent, name, path, len); ent->mode = mode; ent->len = len; hashcpy(ent->sha1, sha1); - memcpy(ent->name, path, len+1); + + ALLOC_GROW(entries, used + 1, alloc); + entries[used++] = ent; } static int ent_compare(const void *a_, const void *b_) diff --git a/builtin/reflog.c b/builtin/reflog.c index 9980731ee7..2d46b6482a 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -382,11 +382,9 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus { struct collected_reflog *e; struct collect_reflog_cb *cb = cb_data; - size_t namelen = strlen(ref); - e = xmalloc(sizeof(*e) + namelen + 1); + FLEX_ALLOC_STR(e, reflog, ref); hashcpy(e->sha1, oid->hash); - memcpy(e->reflog, ref, namelen + 1); ALLOC_GROW(cb->e, cb->nr + 1, cb->alloc); cb->e[cb->nr++] = e; return 0; @@ -411,8 +409,7 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len) ent->pattern[len] == '\0') return ent; - ent = xcalloc(1, sizeof(*ent) + len + 1); - memcpy(ent->pattern, pattern, len); + FLEX_ALLOC_MEM(ent, pattern, pattern, len); *reflog_expire_cfg_tail = ent; reflog_expire_cfg_tail = &(ent->next); return ent; diff --git a/cache-tree.c b/cache-tree.c index a59e6f1e1f..1fbe79a003 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -79,11 +79,9 @@ static struct cache_tree_sub *find_subtree(struct cache_tree *it, ALLOC_GROW(it->down, it->subtree_nr + 1, it->subtree_alloc); it->subtree_nr++; - down = xmalloc(sizeof(*down) + pathlen + 1); + FLEX_ALLOC_MEM(down, name, path, pathlen); down->cache_tree = NULL; down->namelen = pathlen; - memcpy(down->name, path, pathlen); - down->name[pathlen] = 0; if (pos < it->subtree_nr) memmove(it->down + pos + 1, diff --git a/combine-diff.c b/combine-diff.c index 890c4157bd..be09a2b25c 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 { diff --git a/diff.c b/diff.c index 2136b6970b..27d14a7d15 100644 --- a/diff.c +++ b/diff.c @@ -2607,12 +2607,9 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, struct diff_filespec *alloc_filespec(const char *path) { - int namelen = strlen(path); - struct diff_filespec *spec = xmalloc(sizeof(*spec) + namelen + 1); + struct diff_filespec *spec; - memset(spec, 0, sizeof(*spec)); - spec->path = (char *)(spec + 1); - memcpy(spec->path, path, namelen+1); + FLEXPTR_ALLOC_STR(spec, path, path); spec->count = 1; spec->is_binary = -1; return spec; diff --git a/dir.c b/dir.c index cb5bff8d8f..c1bc538a56 100644 --- a/dir.c +++ b/dir.c @@ -503,12 +503,7 @@ void add_exclude(const char *string, const char *base, parse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen); if (flags & EXC_FLAG_MUSTBEDIR) { - char *s; - x = xmalloc(sizeof(*x) + patternlen + 1); - s = (char *)(x+1); - memcpy(s, string, patternlen); - s[patternlen] = '\0'; - x->pattern = s; + FLEXPTR_ALLOC_MEM(x, pattern, string, patternlen); } else { x = xmalloc(sizeof(*x)); x->pattern = string; @@ -625,10 +620,7 @@ static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc, } uc->dir_created++; - d = xmalloc(sizeof(*d) + len + 1); - memset(d, 0, sizeof(*d)); - memcpy(d->name, name, len); - d->name[len] = '\0'; + FLEX_ALLOC_MEM(d, name, name, len); ALLOC_GROW(dir->dirs, dir->dirs_nr + 1, dir->dirs_alloc); memmove(dir->dirs + first + 1, dir->dirs + first, @@ -1167,10 +1159,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len) { struct dir_entry *ent; - ent = xmalloc(sizeof(*ent) + len + 1); + FLEX_ALLOC_MEM(ent, name, pathname, len); ent->len = len; - memcpy(ent->name, pathname, len); - ent->name[len] = 0; return ent; } diff --git a/hashmap.c b/hashmap.c index f693839cb4..b10b642229 100644 --- a/hashmap.c +++ b/hashmap.c @@ -256,10 +256,9 @@ const void *memintern(const void *data, size_t len) e = hashmap_get(&map, &key, data); if (!e) { /* not found: create it */ - e = xmallocz(sizeof(struct pool_entry) + len); + FLEX_ALLOC_MEM(e, data, data, len); hashmap_entry_init(e, key.ent.hash); e->len = len; - memcpy(e->data, data, len); hashmap_add(&map, e); } return e->data; diff --git a/help.c b/help.c index d996b34066..19328ea992 100644 --- a/help.c +++ b/help.c @@ -11,11 +11,9 @@ void add_cmdname(struct cmdnames *cmds, const char *name, int len) { - struct cmdname *ent = xmalloc(sizeof(*ent) + len + 1); - + struct cmdname *ent; + FLEX_ALLOC_MEM(ent, name, name, len); ent->len = len; - memcpy(ent->name, name, len); - ent->name[len] = 0; ALLOC_GROW(cmds->names, cmds->cnt + 1, cmds->alloc); cmds->names[cmds->cnt++] = ent; diff --git a/log-tree.c b/log-tree.c index f70a30e127..60f983934d 100644 --- a/log-tree.c +++ b/log-tree.c @@ -77,9 +77,8 @@ int parse_decorate_color_config(const char *var, const char *slot_name, const ch void add_name_decoration(enum decoration_type type, const char *name, struct object *obj) { - int nlen = strlen(name); - struct name_decoration *res = xmalloc(sizeof(*res) + nlen + 1); - memcpy(res->name, name, nlen + 1); + struct name_decoration *res; + FLEX_ALLOC_STR(res, name, name); res->type = type; res->next = add_decoration(&name_decoration, obj, res); } diff --git a/name-hash.c b/name-hash.c index 332ba956e7..6d9f23e932 100644 --- a/name-hash.c +++ b/name-hash.c @@ -55,10 +55,9 @@ static struct dir_entry *hash_dir_entry(struct index_state *istate, dir = find_dir_entry(istate, ce->name, namelen); if (!dir) { /* not found, create it and add to hash table */ - dir = xcalloc(1, sizeof(struct dir_entry) + namelen + 1); + FLEX_ALLOC_MEM(dir, name, ce->name, namelen); hashmap_entry_init(dir, memihash(ce->name, namelen)); dir->namelen = namelen; - strncpy(dir->name, ce->name, namelen); hashmap_add(&istate->dir_hash, dir); /* recursively add missing parent directories */ diff --git a/ref-filter.c b/ref-filter.c index f097176ed9..9ccfc51cea 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1255,10 +1255,8 @@ static struct ref_array_item *new_ref_array_item(const char *refname, const unsigned char *objectname, int flag) { - size_t len = strlen(refname); - struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1); - memcpy(ref->refname, refname, len); - ref->refname[len] = '\0'; + struct ref_array_item *ref; + FLEX_ALLOC_STR(ref, refname, refname); hashcpy(ref->objectname, objectname); ref->flag = flag; diff --git a/refs.c b/refs.c index 1d9e2a7932..2d86445231 100644 --- a/refs.c +++ b/refs.c @@ -761,10 +761,8 @@ void ref_transaction_free(struct ref_transaction *transaction) static struct ref_update *add_update(struct ref_transaction *transaction, const char *refname) { - size_t len = strlen(refname) + 1; - struct ref_update *update = xcalloc(1, sizeof(*update) + len); - - memcpy((char *)update->refname, refname, len); /* includes NUL */ + struct ref_update *update; + FLEX_ALLOC_STR(update, refname, refname); ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc); transaction->updates[transaction->nr++] = update; return update; diff --git a/refs/files-backend.c b/refs/files-backend.c index 3a27f27585..de9af1615c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -199,17 +199,14 @@ static struct ref_entry *create_ref_entry(const char *refname, const unsigned char *sha1, int flag, int check_name) { - int len; struct ref_entry *ref; if (check_name && check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) die("Reference has invalid format: '%s'", refname); - len = strlen(refname) + 1; - ref = xmalloc(sizeof(struct ref_entry) + len); + FLEX_ALLOC_STR(ref, name, refname); hashcpy(ref->u.value.oid.hash, sha1); oidclr(&ref->u.value.peeled); - memcpy(ref->name, refname, len); ref->flag = flag; return ref; } @@ -268,9 +265,7 @@ static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache, int incomplete) { struct ref_entry *direntry; - direntry = xcalloc(1, sizeof(struct ref_entry) + len + 1); - memcpy(direntry->name, dirname, len); - direntry->name[len] = '\0'; + FLEX_ALLOC_MEM(direntry, name, dirname, len); direntry->u.subdir.ref_cache = ref_cache; direntry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0); return direntry; @@ -939,13 +934,10 @@ static void clear_loose_ref_cache(struct ref_cache *refs) */ static struct ref_cache *create_ref_cache(const char *submodule) { - int len; struct ref_cache *refs; if (!submodule) submodule = ""; - len = strlen(submodule) + 1; - refs = xcalloc(1, sizeof(struct ref_cache) + len); - memcpy(refs->name, submodule, len); + FLEX_ALLOC_STR(refs, name, submodule); refs->next = submodule_ref_caches; submodule_ref_caches = refs; return refs; @@ -2191,10 +2183,9 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data) /* Schedule the loose reference for pruning if requested. */ if ((cb->flags & PACK_REFS_PRUNE)) { - int namelen = strlen(entry->name) + 1; - struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen); + struct ref_to_prune *n; + FLEX_ALLOC_STR(n, name, entry->name); hashcpy(n->sha1, entry->u.value.oid.hash); - memcpy(n->name, entry->name, namelen); /* includes NUL */ n->next = cb->ref_to_prune; cb->ref_to_prune = n; } diff --git a/remote.c b/remote.c index 9d34b5a5da..7a8a8a1b4a 100644 --- a/remote.c +++ b/remote.c @@ -2132,16 +2132,13 @@ static int one_local_ref(const char *refname, const struct object_id *oid, { struct ref ***local_tail = cb_data; struct ref *ref; - int len; /* we already know it starts with refs/ to get here */ if (check_refname_format(refname + 5, 0)) return 0; - len = strlen(refname) + 1; - ref = xcalloc(1, sizeof(*ref) + len); + ref = alloc_ref(refname); oidcpy(&ref->new_oid, oid); - memcpy(ref->name, refname, len); **local_tail = ref; *local_tail = &ref->next; return 0; -- cgit v1.3-5-g9baa 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 --- archive.c | 4 ++-- builtin/apply.c | 2 +- builtin/clean.c | 2 +- builtin/fetch.c | 2 +- builtin/index-pack.c | 4 ++-- builtin/merge.c | 2 +- builtin/mv.c | 4 ++-- builtin/receive-pack.c | 2 +- combine-diff.c | 14 +++++++------- commit.c | 2 +- compat/mingw.c | 4 ++-- compat/qsort.c | 2 +- compat/setenv.c | 2 +- compat/win32/syslog.c | 4 ++-- diffcore-delta.c | 6 ++++-- diffcore-rename.c | 2 +- dir.c | 4 ++-- fast-import.c | 2 +- refs.c | 2 +- remote.c | 8 ++++---- revision.c | 2 +- sha1_file.c | 20 +++++++++++--------- sha1_name.c | 5 ++--- shallow.c | 2 +- submodule.c | 6 +++--- 25 files changed, 56 insertions(+), 53 deletions(-) (limited to 'builtin') diff --git a/archive.c b/archive.c index 0687afae43..5d735ae603 100644 --- a/archive.c +++ b/archive.c @@ -171,8 +171,8 @@ static void queue_directory(const unsigned char *sha1, unsigned mode, int stage, struct archiver_context *c) { struct directory *d; - size_t len = base->len + 1 + strlen(filename) + 1; - d = xmalloc(sizeof(*d) + len); + size_t len = st_add4(base->len, 1, strlen(filename), 1); + d = xmalloc(st_add(sizeof(*d), len)); d->up = c->bottom; d->baselen = base->len; d->mode = mode; diff --git a/builtin/apply.c b/builtin/apply.c index deb1364fa8..0db6d14cc2 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -2632,7 +2632,7 @@ static void update_image(struct image *img, insert_count = postimage->len; /* Adjust the contents */ - result = xmalloc(img->len + insert_count - remove_count + 1); + result = xmalloc(st_add3(st_sub(img->len, remove_count), insert_count, 1)); memcpy(result, img->buf, applied_at); memcpy(result + applied_at, postimage->buf, postimage->len); memcpy(result + applied_at + postimage->len, diff --git a/builtin/clean.c b/builtin/clean.c index 1f02c8294c..fb1824ce95 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -615,7 +615,7 @@ static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff) nr += chosen[i]; } - result = xcalloc(nr + 1, sizeof(int)); + result = xcalloc(st_add(nr, 1), sizeof(int)); for (i = 0; i < stuff->nr && j < nr; i++) { if (chosen[i]) result[j++] = i; diff --git a/builtin/fetch.c b/builtin/fetch.c index 17f40e10f6..683f08ec91 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1107,7 +1107,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv) if (argc > 0) { int j = 0; int i; - refs = xcalloc(argc + 1, sizeof(const char *)); + refs = xcalloc(st_add(argc, 1), sizeof(const char *)); for (i = 0; i < argc; i++) { if (!strcmp(argv[i], "tag")) { i++; diff --git a/builtin/index-pack.c b/builtin/index-pack.c index a60bcfac06..193908a619 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1744,9 +1744,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) curr_pack = open_pack_file(pack_name); parse_pack_header(); - objects = xcalloc(nr_objects + 1, sizeof(struct object_entry)); + objects = xcalloc(st_add(nr_objects, 1), sizeof(struct object_entry)); if (show_stat) - obj_stat = xcalloc(nr_objects + 1, sizeof(struct object_stat)); + obj_stat = xcalloc(st_add(nr_objects, 1), sizeof(struct object_stat)); ofs_deltas = xcalloc(nr_objects, sizeof(struct ofs_delta_entry)); parse_pack_objects(pack_sha1); resolve_deltas(); diff --git a/builtin/merge.c b/builtin/merge.c index b98a3489bf..101ffeff4c 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -939,7 +939,7 @@ static int setup_with_upstream(const char ***argv) if (!branch->merge_nr) die(_("No default upstream defined for the current branch.")); - args = xcalloc(branch->merge_nr + 1, sizeof(char *)); + args = xcalloc(st_add(branch->merge_nr, 1), sizeof(char *)); for (i = 0; i < branch->merge_nr; i++) { if (!branch->merge[i]->dst) die(_("No remote-tracking branch for %s from %s"), diff --git a/builtin/mv.c b/builtin/mv.c index 9a9813a0ec..aeae855e2b 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -48,9 +48,9 @@ static const char **internal_copy_pathspec(const char *prefix, static const char *add_slash(const char *path) { - int len = strlen(path); + size_t len = strlen(path); if (path[len - 1] != '/') { - char *with_slash = xmalloc(len + 2); + char *with_slash = xmalloc(st_add(len, 2)); memcpy(with_slash, path, len); with_slash[len++] = '/'; with_slash[len] = 0; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 3dc3868c86..c8e32b297c 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1372,7 +1372,7 @@ static struct command **queue_command(struct command **tail, refname = line + 82; reflen = linelen - 82; - cmd = xcalloc(1, sizeof(struct command) + reflen + 1); + cmd = xcalloc(1, st_add3(sizeof(struct command), reflen, 1)); hashcpy(cmd->old_sha1, old_sha1); hashcpy(cmd->new_sha1, new_sha1); memcpy(cmd->ref_name, refname, reflen); diff --git a/combine-diff.c b/combine-diff.c index be09a2b25c..0e1d4b0893 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; diff --git a/commit.c b/commit.c index 31cd91f81e..3f4f371e5e 100644 --- a/commit.c +++ b/commit.c @@ -147,7 +147,7 @@ struct commit_graft *read_graft_line(char *buf, int len) if ((len + 1) % entry_size) goto bad_graft_data; i = (len + 1) / entry_size - 1; - graft = xmalloc(sizeof(*graft) + GIT_SHA1_RAWSZ * i); + graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i))); graft->nr_parent = i; if (get_oid_hex(buf, &graft->oid)) goto bad_graft_data; diff --git a/compat/mingw.c b/compat/mingw.c index d8a5345a30..ae16d089ad 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -769,7 +769,7 @@ static const char *quote_arg(const char *arg) return arg; /* insert \ where necessary */ - d = q = xmalloc(len+n+3); + d = q = xmalloc(st_add3(len, n, 3)); *d++ = '"'; while (*arg) { if (*arg == '"') @@ -1028,7 +1028,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen free(quoted); } - wargs = xmalloc((2 * args.len + 1) * sizeof(wchar_t)); + wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t)); xutftowcs(wargs, args.buf, 2 * args.len + 1); strbuf_release(&args); diff --git a/compat/qsort.c b/compat/qsort.c index 9574d537bd..7d071afb70 100644 --- a/compat/qsort.c +++ b/compat/qsort.c @@ -47,7 +47,7 @@ static void msort_with_tmp(void *b, size_t n, size_t s, void git_qsort(void *b, size_t n, size_t s, int (*cmp)(const void *, const void *)) { - const size_t size = n * s; + const size_t size = st_mult(n, s); char buf[1024]; if (size < sizeof(buf)) { diff --git a/compat/setenv.c b/compat/setenv.c index fc1439a643..7849f258d2 100644 --- a/compat/setenv.c +++ b/compat/setenv.c @@ -18,7 +18,7 @@ int gitsetenv(const char *name, const char *value, int replace) namelen = strlen(name); valuelen = strlen(value); - envstr = malloc((namelen + valuelen + 2)); + envstr = malloc(st_add3(namelen, valuelen, 2)); if (!envstr) { errno = ENOMEM; return -1; diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c index d015e436d5..b905aea31b 100644 --- a/compat/win32/syslog.c +++ b/compat/win32/syslog.c @@ -32,7 +32,7 @@ void syslog(int priority, const char *fmt, ...) return; } - str = malloc(str_len + 1); + str = malloc(st_add(str_len, 1)); if (!str) { warning("malloc failed: '%s'", strerror(errno)); return; @@ -43,7 +43,7 @@ void syslog(int priority, const char *fmt, ...) va_end(ap); while ((pos = strstr(str, "%1")) != NULL) { - str = realloc(str, ++str_len + 1); + str = realloc(str, st_add(++str_len, 1)); if (!str) { warning("realloc failed: '%s'", strerror(errno)); return; diff --git a/diffcore-delta.c b/diffcore-delta.c index 7cf431d261..4159748a70 100644 --- a/diffcore-delta.c +++ b/diffcore-delta.c @@ -53,7 +53,8 @@ static struct spanhash_top *spanhash_rehash(struct spanhash_top *orig) int osz = 1 << orig->alloc_log2; int sz = osz << 1; - new = xmalloc(sizeof(*orig) + sizeof(struct spanhash) * sz); + new = xmalloc(st_add(sizeof(*orig), + st_mult(sizeof(struct spanhash), sz))); new->alloc_log2 = orig->alloc_log2 + 1; new->free = INITIAL_FREE(new->alloc_log2); memset(new->data, 0, sizeof(struct spanhash) * sz); @@ -130,7 +131,8 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one) int is_text = !diff_filespec_is_binary(one); i = INITIAL_HASH_SIZE; - hash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1<alloc_log2 = i; hash->free = INITIAL_FREE(i); memset(hash->data, 0, sizeof(struct spanhash) * (1< rd->end) return -1; - *untracked_ = untracked = xmalloc(sizeof(*untracked) + len); + *untracked_ = untracked = xmalloc(st_add(sizeof(*untracked), len)); memcpy(untracked, &ud, sizeof(ud)); memcpy(untracked->name, data, len + 1); data = next; diff --git a/fast-import.c b/fast-import.c index 36dbaa5b16..9622b89df7 100644 --- a/fast-import.c +++ b/fast-import.c @@ -622,7 +622,7 @@ static void *pool_alloc(size_t len) return xmalloc(len); } total_allocd += sizeof(struct mem_pool) + mem_pool_alloc; - p = xmalloc(sizeof(struct mem_pool) + mem_pool_alloc); + p = xmalloc(st_add(sizeof(struct mem_pool), mem_pool_alloc)); p->next_pool = mem_pool; p->next_free = (char *) p->space; p->end = p->next_free + mem_pool_alloc; diff --git a/refs.c b/refs.c index 2d86445231..b0e6ece6f4 100644 --- a/refs.c +++ b/refs.c @@ -906,7 +906,7 @@ char *shorten_unambiguous_ref(const char *refname, int strict) /* -2 for strlen("%.*s") - strlen("%s"); +1 for NUL */ total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 + 1; - scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len); + scanf_fmts = xmalloc(st_add(st_mult(nr_rules, sizeof(char *)), total_len)); offset = 0; for (i = 0; i < nr_rules; i++) { diff --git a/remote.c b/remote.c index 7a8a8a1b4a..f182382c83 100644 --- a/remote.c +++ b/remote.c @@ -928,7 +928,7 @@ static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen, const char *name) { size_t len = strlen(name); - struct ref *ref = xcalloc(1, sizeof(struct ref) + prefixlen + len + 1); + struct ref *ref = xcalloc(1, st_add4(sizeof(*ref), prefixlen, len, 1)); memcpy(ref->name, prefix, prefixlen); memcpy(ref->name + prefixlen, name, len); return ref; @@ -945,9 +945,9 @@ struct ref *copy_ref(const struct ref *ref) size_t len; if (!ref) return NULL; - len = strlen(ref->name); - cpy = xmalloc(sizeof(struct ref) + len + 1); - memcpy(cpy, ref, sizeof(struct ref) + len + 1); + len = st_add3(sizeof(struct ref), strlen(ref->name), 1); + cpy = xmalloc(len); + memcpy(cpy, ref, len); cpy->next = NULL; cpy->symref = xstrdup_or_null(ref->symref); cpy->remote_status = xstrdup_or_null(ref->remote_status); diff --git a/revision.c b/revision.c index 14daefb174..df56fcea0e 100644 --- a/revision.c +++ b/revision.c @@ -540,7 +540,7 @@ struct treesame_state { static struct treesame_state *initialise_treesame(struct rev_info *revs, struct commit *commit) { unsigned n = commit_list_count(commit->parents); - struct treesame_state *st = xcalloc(1, sizeof(*st) + n); + struct treesame_state *st = xcalloc(1, st_add(sizeof(*st), n)); st->nparents = n; add_decoration(&revs->treesame, &commit->object, st); return st; diff --git a/sha1_file.c b/sha1_file.c index d14abfed9b..ab16c7b98c 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -253,7 +253,7 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base, { struct alternate_object_database *ent; struct alternate_object_database *alt; - int pfxlen, entlen; + size_t pfxlen, entlen; struct strbuf pathbuf = STRBUF_INIT; if (!is_absolute_path(entry) && relative_base) { @@ -273,8 +273,8 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base, while (pfxlen && pathbuf.buf[pfxlen-1] == '/') pfxlen -= 1; - entlen = pfxlen + 43; /* '/' + 2 hex + '/' + 38 hex + NUL */ - ent = xmalloc(sizeof(*ent) + entlen); + entlen = st_add(pfxlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */ + ent = xmalloc(st_add(sizeof(*ent), entlen)); memcpy(ent->base, pathbuf.buf, pfxlen); strbuf_release(&pathbuf); @@ -1134,7 +1134,7 @@ unsigned char *use_pack(struct packed_git *p, static struct packed_git *alloc_packed_git(int extra) { - struct packed_git *p = xmalloc(sizeof(*p) + extra); + struct packed_git *p = xmalloc(st_add(sizeof(*p), extra)); memset(p, 0, sizeof(*p)); p->pack_fd = -1; return p; @@ -1168,7 +1168,7 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local) * ".pack" is long enough to hold any suffix we're adding (and * the use xsnprintf double-checks that) */ - alloc = path_len + strlen(".pack") + 1; + alloc = st_add3(path_len, strlen(".pack"), 1); p = alloc_packed_git(alloc); memcpy(p->pack_name, path, path_len); @@ -1196,7 +1196,7 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local) struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path) { const char *path = sha1_pack_name(sha1); - int alloc = strlen(path) + 1; + size_t alloc = st_add(strlen(path), 1); struct packed_git *p = alloc_packed_git(alloc); memcpy(p->pack_name, path, alloc); /* includes NUL */ @@ -1413,10 +1413,12 @@ static void mark_bad_packed_object(struct packed_git *p, { unsigned i; for (i = 0; i < p->num_bad_objects; i++) - if (!hashcmp(sha1, p->bad_object_sha1 + 20 * i)) + if (!hashcmp(sha1, p->bad_object_sha1 + GIT_SHA1_RAWSZ * i)) return; - p->bad_object_sha1 = xrealloc(p->bad_object_sha1, 20 * (p->num_bad_objects + 1)); - hashcpy(p->bad_object_sha1 + 20 * p->num_bad_objects, sha1); + p->bad_object_sha1 = xrealloc(p->bad_object_sha1, + st_mult(GIT_SHA1_RAWSZ, + st_add(p->num_bad_objects, 1))); + hashcpy(p->bad_object_sha1 + GIT_SHA1_RAWSZ * p->num_bad_objects, sha1); p->num_bad_objects++; } diff --git a/sha1_name.c b/sha1_name.c index 892db21813..532db4fdc6 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -87,9 +87,8 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa * object databases including our own. */ const char *objdir = get_object_directory(); - int objdir_len = strlen(objdir); - int entlen = objdir_len + 43; - fakeent = xmalloc(sizeof(*fakeent) + entlen); + size_t objdir_len = strlen(objdir); + fakeent = xmalloc(st_add3(sizeof(*fakeent), objdir_len, 43)); memcpy(fakeent->base, objdir, objdir_len); fakeent->name = fakeent->base + objdir_len + 1; fakeent->name[-1] = '/'; diff --git a/shallow.c b/shallow.c index 71163bf3d7..4d554caf8d 100644 --- a/shallow.c +++ b/shallow.c @@ -389,7 +389,7 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1, unsigned int i, nr; struct commit_list *head = NULL; int bitmap_nr = (info->nr_bits + 31) / 32; - int bitmap_size = bitmap_nr * sizeof(uint32_t); + size_t bitmap_size = st_mult(bitmap_nr, sizeof(uint32_t)); uint32_t *tmp = xmalloc(bitmap_size); /* to be freed before return */ uint32_t *bitmap = paint_alloc(info); struct commit *c = lookup_commit_reference_gently(sha1, 1); diff --git a/submodule.c b/submodule.c index 14e76247bf..bd97b15a92 100644 --- a/submodule.c +++ b/submodule.c @@ -122,7 +122,7 @@ static int add_submodule_odb(const char *path) struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; int ret = 0; - int alloc; + size_t alloc; strbuf_git_path_submodule(&objects_directory, path, "objects/"); if (!is_directory(objects_directory.buf)) { @@ -137,8 +137,8 @@ static int add_submodule_odb(const char *path) objects_directory.len)) goto done; - alloc = objects_directory.len + 42; /* for "12/345..." sha1 */ - alt_odb = xmalloc(sizeof(*alt_odb) + alloc); + alloc = st_add(objects_directory.len, 42); /* for "12/345..." sha1 */ + alt_odb = xmalloc(st_add(sizeof(*alt_odb), alloc)); alt_odb->next = alt_odb_list; xsnprintf(alt_odb->base, alloc, "%s", objects_directory.buf); alt_odb->name = alt_odb->base + objects_directory.len; -- cgit v1.3-5-g9baa From 5545f057d4684de99feca0ca480297e3d71fb812 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:44:50 -0500 Subject: fetch-pack: simplify add_sought_entry We have two variants of this function, one that takes a string and one that takes a ptr/len combo. But we only call the latter with the length of a NUL-terminated string, so our first simplification is to drop it in favor of the string variant. Since we know we have a string, we can also replace the manual memory computation with a call to alloc_ref(). Furthermore, we can rely on get_oid_hex() to complain if it hits the end of the string. That means we can simplify the check for " " versus just "". Rather than manage the ptr/len pair, we can just bump the start of our string forward. The original code over-allocated based on the original "namelen" (which wasn't _wrong_, but was simply wasteful and confusing). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) (limited to 'builtin') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index cf3019e05b..7d5914f921 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -10,33 +10,24 @@ static const char fetch_pack_usage[] = "[--include-tag] [--upload-pack=] [--depth=] " "[--no-progress] [--diag-url] [-v] [:] [...]"; -static void add_sought_entry_mem(struct ref ***sought, int *nr, int *alloc, - const char *name, int namelen) +static void add_sought_entry(struct ref ***sought, int *nr, int *alloc, + const char *name) { - struct ref *ref = xcalloc(1, sizeof(*ref) + namelen + 1); + struct ref *ref; struct object_id oid; - const int chunksz = GIT_SHA1_HEXSZ + 1; - if (namelen > chunksz && name[chunksz - 1] == ' ' && - !get_oid_hex(name, &oid)) { - oidcpy(&ref->old_oid, &oid); - name += chunksz; - namelen -= chunksz; - } + if (!get_oid_hex(name, &oid) && name[GIT_SHA1_HEXSZ] == ' ') + name += GIT_SHA1_HEXSZ + 1; + else + oidclr(&oid); - memcpy(ref->name, name, namelen); - ref->name[namelen] = '\0'; + ref = alloc_ref(name); + oidcpy(&ref->old_oid, &oid); (*nr)++; ALLOC_GROW(*sought, *nr, *alloc); (*sought)[*nr - 1] = ref; } -static void add_sought_entry(struct ref ***sought, int *nr, int *alloc, - const char *string) -{ - add_sought_entry_mem(sought, nr, alloc, string, strlen(string)); -} - int cmd_fetch_pack(int argc, const char **argv, const char *prefix) { int i, ret; -- cgit v1.3-5-g9baa