Re: [PATCH 2/2] use CALLOC_ARRAY
From: René Scharfe. <hidden>
Date: 2021-03-15 22:35:50
Am 15.03.21 um 20:26 schrieb Junio C Hamano:
https://github.com/git/git/runs/2114848725?check_suite_focus=true tells us that we want to be using CALLOC_ARRAY() instead of xcalloc() at these places. Quite a many are single-element allocations that want NUL-filled piece of memory. We may want to resurrect the "one-element calloc is exempt" thing (in other words, xcalloc(1, size_of_one_element) is perfectly fine, even though xcalloc(size_of_one_element, 1) must swap the order of arguments).
Leaving the idiomatic use of xcalloc for allocating a single cleared object alone can make sense. CALLOC_ARRAY with a single element might become idiomatic over time as well, though, but with "C" (for continuous) and "ARRAY" having two name parts screaming for multiple elements might be a bit much.
But some are true multi-element allocations that we may want to fix
in each topic in flight.
compat/simple-ipc/ipc-unix-socket.c::836
1e7cdcb6 (simple-ipc: add Unix domain socket implementation, 2021-03-09)
merge-strategies.c::498
72a74644 (merge-octopus: rewrite in C, 2020-11-24)
t/helper/test-bloom.c::72
f1294eaf (bloom.c: introduce core Bloom filter constructs, 2020-03-30)
Also there are a few multi-element allocations that are old, which
weren't somehow caught by the patch I am responding to.When I run coccicheck agains seen it still misses the following conversions: builtin/checkout.c | 2 +- builtin/log.c | 4 ++-- builtin/receive-pack.c | 10 ++++------ t/helper/test-bloom.c | 2 +- That's with spatch built from the latest Coccinelle source as of March 2nd. Strange.
quoted hunk
builtin/receive-pack.c::2351 0a1bc12b (receive-pack: allow pushes that update .git/shallow, 2013-12-05) ----- >8 ---------- >8 ---------- >8 ---------- >8 ---------- >8 ----- GIT_VERSION = 2.31.0.rc2.390.g46dd778bfd SPATCH contrib/coccinelle/hashmap.cocci SPATCH contrib/coccinelle/commit.cocci SPATCH contrib/coccinelle/xcalloc.cocci SPATCH contrib/coccinelle/preincr.cocci SPATCH contrib/coccinelle/free.cocci SPATCH contrib/coccinelle/qsort.cocci SPATCH contrib/coccinelle/object_id.cocci SPATCH contrib/coccinelle/xstrdup_or_null.cocci SPATCH contrib/coccinelle/swap.cocci SPATCH contrib/coccinelle/strbuf.cocci SPATCH contrib/coccinelle/flex_alloc.cocci SPATCH contrib/coccinelle/array.cocci SPATCH result: contrib/coccinelle/array.cocci.patch + set +x Coccinelle suggests the following changes in 'contrib/coccinelle/array.cocci.patch': diff -u -p a/builtin/checkout.c b/builtin/checkout.c--- a/builtin/checkout.c +++ b/builtin/checkout.c@@ -741,7 +741,7 @@ static int merge_working_tree(const stru &new_branch_info->commit->object.oid : &new_branch_info->oid, NULL); if (opts->overwrite_ignore) { - topts.dir = xcalloc(1, sizeof(*topts.dir)); + CALLOC_ARRAY(topts.dir, 1); topts.dir->flags |= DIR_SHOW_IGNORED; setup_standard_excludes(topts.dir); }diff -u -p a/builtin/log.c b/builtin/log.c--- a/builtin/log.c +++ b/builtin/log.c@@ -230,7 +230,7 @@ static void cmd_log_init_finish(int argc } if (mailmap) { - rev->mailmap = xcalloc(1, sizeof(struct string_list)); + CALLOC_ARRAY(rev->mailmap, 1); read_mailmap(rev->mailmap); }@@ -2141,7 +2141,7 @@ int cmd_format_patch(int argc, const cha } if (in_reply_to || thread || cover_letter) - rev.ref_message_ids = xcalloc(1, sizeof(struct string_list)); + CALLOC_ARRAY(rev.ref_message_ids, 1); if (in_reply_to) { const char *msgid = clean_message_id(in_reply_to); string_list_append(rev.ref_message_ids, msgid);diff -u -p a/builtin/receive-pack.c b/builtin/receive-pack.c--- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c@@ -1040,7 +1040,7 @@ static int read_proc_receive_report(stru report = hint->report; while (report->next) report = report->next; - report->next = xcalloc(1, sizeof(struct ref_push_report)); + CALLOC_ARRAY(report->next, 1); report = report->next; } new_report = 0;@@ -2348,11 +2348,9 @@ static void prepare_shallow_update(struc ALLOC_ARRAY(si->used_shallow, si->shallow->nr); assign_shallow_commits_to_refs(si, si->used_shallow, NULL); - si->need_reachability_test = - xcalloc(si->shallow->nr, sizeof(*si->need_reachability_test)); - si->reachable = - xcalloc(si->shallow->nr, sizeof(*si->reachable)); - si->shallow_ref = xcalloc(si->ref->nr, sizeof(*si->shallow_ref)); + CALLOC_ARRAY(si->need_reachability_test, si->shallow->nr); + CALLOC_ARRAY(si->reachable, si->shallow->nr); + CALLOC_ARRAY(si->shallow_ref, si->ref->nr); for (i = 0; i < si->nr_ours; i++) si->need_reachability_test[si->ours[i]] = 1;diff -u -p a/builtin/repack.c b/builtin/repack.c--- a/builtin/repack.c +++ b/builtin/repack.c@@ -327,7 +327,7 @@ static void init_pack_geometry(struct pa struct packed_git *p; struct pack_geometry *geometry; - *geometry_p = xcalloc(1, sizeof(struct pack_geometry)); + CALLOC_ARRAY(*geometry_p, 1); geometry = *geometry_p; for (p = get_all_packs(the_repository); p; p = p->next) {diff -u -p a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c--- a/compat/simple-ipc/ipc-unix-socket.c +++ b/compat/simple-ipc/ipc-unix-socket.c@@ -130,7 +130,7 @@ enum ipc_active_state ipc_client_try_con trace2_region_leave("ipc-client", "try-connect", NULL); if (state == IPC_STATE__LISTENING) { - (*p_connection) = xcalloc(1, sizeof(struct ipc_client_connection)); + CALLOC_ARRAY((*p_connection), 1); (*p_connection)->fd = fd; }@@ -819,7 +819,7 @@ int ipc_server_run_async(struct ipc_serv return ret; } - server_data = xcalloc(1, sizeof(*server_data)); + CALLOC_ARRAY(server_data, 1); server_data->magic = MAGIC_SERVER_DATA; server_data->application_cb = application_cb; server_data->application_data = application_data;@@ -833,11 +833,9 @@ int ipc_server_run_async(struct ipc_serv pthread_cond_init(&server_data->work_available_cond, NULL); server_data->queue_size = nr_threads * FIFO_SCALE; - server_data->fifo_fds = xcalloc(server_data->queue_size, - sizeof(*server_data->fifo_fds)); + CALLOC_ARRAY(server_data->fifo_fds, server_data->queue_size); - server_data->accept_thread = - xcalloc(1, sizeof(*server_data->accept_thread)); + CALLOC_ARRAY(server_data->accept_thread, 1); server_data->accept_thread->magic = MAGIC_ACCEPT_THREAD_DATA; server_data->accept_thread->server_data = server_data; server_data->accept_thread->server_socket = server_socket;@@ -851,7 +849,7 @@ int ipc_server_run_async(struct ipc_serv for (k = 0; k < nr_threads; k++) { struct ipc_worker_thread_data *wtd; - wtd = xcalloc(1, sizeof(*wtd)); + CALLOC_ARRAY(wtd, 1); wtd->magic = MAGIC_WORKER_THREAD_DATA; wtd->server_data = server_data;diff -u -p a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c--- a/compat/simple-ipc/ipc-win32.c +++ b/compat/simple-ipc/ipc-win32.c@@ -184,7 +184,7 @@ enum ipc_active_state ipc_client_try_con trace2_region_leave("ipc-client", "try-connect", NULL); if (state == IPC_STATE__LISTENING) { - (*p_connection) = xcalloc(1, sizeof(struct ipc_client_connection)); + CALLOC_ARRAY((*p_connection), 1); (*p_connection)->fd = fd; }@@ -625,7 +625,7 @@ int ipc_server_run_async(struct ipc_serv return -2; } - server_data = xcalloc(1, sizeof(*server_data)); + CALLOC_ARRAY(server_data, 1); server_data->magic = MAGIC_SERVER_DATA; server_data->application_cb = application_cb; server_data->application_data = application_data;@@ -640,7 +640,7 @@ int ipc_server_run_async(struct ipc_serv for (k = 0; k < nr_threads; k++) { struct ipc_server_thread_data *std; - std = xcalloc(1, sizeof(*std)); + CALLOC_ARRAY(std, 1); std->magic = MAGIC_SERVER_THREAD_DATA; std->server_data = server_data; std->hPipe = INVALID_HANDLE_VALUE;diff -u -p a/merge-ort.c b/merge-ort.c--- a/merge-ort.c +++ b/merge-ort.c@@ -3439,7 +3439,7 @@ static void merge_start(struct merge_opt trace2_region_leave("merge", "allocate/init", opt->repo); return; } - opt->priv = xcalloc(1, sizeof(*opt->priv)); + CALLOC_ARRAY(opt->priv, 1); /* Initialization of various renames fields */ renames = &opt->priv->renames;diff -u -p a/merge-strategies.c b/merge-strategies.c--- a/merge-strategies.c +++ b/merge-strategies.c@@ -495,8 +495,7 @@ int merge_strategies_octopus(struct repo return 2; } - reference_commit = xcalloc(commit_list_count(remotes) + 1, - sizeof(struct commit *)); + CALLOC_ARRAY(reference_commit, commit_list_count(remotes) + 1); reference_commit[0] = head_commit; reference_tree = head_tree;diff -u -p a/t/helper/test-bloom.c b/t/helper/test-bloom.c--- a/t/helper/test-bloom.c +++ b/t/helper/test-bloom.c@@ -69,7 +69,7 @@ int cmd__bloom(int argc, const char **ar struct bloom_filter filter; int i = 2; filter.len = (settings.bits_per_entry + BITS_PER_WORD - 1) / BITS_PER_WORD; - filter.data = xcalloc(filter.len, sizeof(unsigned char)); + CALLOC_ARRAY(filter.data, filter.len); if (argc - 1 < i) usage(bloom_usage);diff -u -p a/unix-stream-server.c b/unix-stream-server.c--- a/unix-stream-server.c +++ b/unix-stream-server.c@@ -72,7 +72,7 @@ int unix_stream_server__create( return -1; } - server_socket = xcalloc(1, sizeof(*server_socket)); + CALLOC_ARRAY(server_socket, 1); server_socket->path_socket = strdup(path); server_socket->fd_socket = fd_socket; lstat(path, &server_socket->st_socket);error: Coccinelle suggested some changes Error: Process completed with exit code 1.