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. > > 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. >
