From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 03/11] commit-graph: avoid malloc'ing a local variable
Date: Thu, 15 May 2025 13:11:41 +0000 [thread overview]
Message-ID: <5a3a8880a68f8c69c2af39d2e32ebb56eb5fa483.1747314709.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1891.git.1747314709.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We do need a context to write the commit graph, but that context is only
needed during the life time of `commit_graph_write()`, therefore it can
easily be a stack variable.
This also helps CodeQL recognize that it is safe to assign the address
of other local variables to the context's fields.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
commit-graph.c | 141 ++++++++++++++++++++++++-------------------------
1 file changed, 69 insertions(+), 72 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 6394752b0b08..9f0115dac9b5 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2509,7 +2509,17 @@ int write_commit_graph(struct object_directory *odb,
const struct commit_graph_opts *opts)
{
struct repository *r = the_repository;
- struct write_commit_graph_context *ctx;
+ struct write_commit_graph_context ctx = {
+ .r = r,
+ .odb = odb,
+ .append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0,
+ .report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0,
+ .split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0,
+ .opts = opts,
+ .total_bloom_filter_data_size = 0,
+ .write_generation_data = (get_configured_generation_version(r) == 2),
+ .num_generation_data_overflows = 0,
+ };
uint32_t i;
int res = 0;
int replace = 0;
@@ -2531,17 +2541,6 @@ int write_commit_graph(struct object_directory *odb,
return 0;
}
- CALLOC_ARRAY(ctx, 1);
- ctx->r = r;
- ctx->odb = odb;
- ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;
- ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;
- ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;
- ctx->opts = opts;
- ctx->total_bloom_filter_data_size = 0;
- ctx->write_generation_data = (get_configured_generation_version(r) == 2);
- ctx->num_generation_data_overflows = 0;
-
bloom_settings.hash_version = r->settings.commit_graph_changed_paths_version;
bloom_settings.bits_per_entry = git_env_ulong("GIT_TEST_BLOOM_SETTINGS_BITS_PER_ENTRY",
bloom_settings.bits_per_entry);
@@ -2549,14 +2548,14 @@ int write_commit_graph(struct object_directory *odb,
bloom_settings.num_hashes);
bloom_settings.max_changed_paths = git_env_ulong("GIT_TEST_BLOOM_SETTINGS_MAX_CHANGED_PATHS",
bloom_settings.max_changed_paths);
- ctx->bloom_settings = &bloom_settings;
+ ctx.bloom_settings = &bloom_settings;
init_topo_level_slab(&topo_levels);
- ctx->topo_levels = &topo_levels;
+ ctx.topo_levels = &topo_levels;
- prepare_commit_graph(ctx->r);
- if (ctx->r->objects->commit_graph) {
- struct commit_graph *g = ctx->r->objects->commit_graph;
+ prepare_commit_graph(ctx.r);
+ if (ctx.r->objects->commit_graph) {
+ struct commit_graph *g = ctx.r->objects->commit_graph;
while (g) {
g->topo_levels = &topo_levels;
@@ -2565,15 +2564,15 @@ int write_commit_graph(struct object_directory *odb,
}
if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
- ctx->changed_paths = 1;
+ ctx.changed_paths = 1;
if (!(flags & COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS)) {
struct commit_graph *g;
- g = ctx->r->objects->commit_graph;
+ g = ctx.r->objects->commit_graph;
/* We have changed-paths already. Keep them in the next graph */
if (g && g->bloom_filter_settings) {
- ctx->changed_paths = 1;
+ ctx.changed_paths = 1;
/* don't propagate the hash_version unless unspecified */
if (bloom_settings.hash_version == -1)
@@ -2586,116 +2585,114 @@ int write_commit_graph(struct object_directory *odb,
bloom_settings.hash_version = bloom_settings.hash_version == 2 ? 2 : 1;
- if (ctx->split) {
- struct commit_graph *g = ctx->r->objects->commit_graph;
+ if (ctx.split) {
+ struct commit_graph *g = ctx.r->objects->commit_graph;
while (g) {
- ctx->num_commit_graphs_before++;
+ ctx.num_commit_graphs_before++;
g = g->base_graph;
}
- if (ctx->num_commit_graphs_before) {
- ALLOC_ARRAY(ctx->commit_graph_filenames_before, ctx->num_commit_graphs_before);
- i = ctx->num_commit_graphs_before;
- g = ctx->r->objects->commit_graph;
+ if (ctx.num_commit_graphs_before) {
+ ALLOC_ARRAY(ctx.commit_graph_filenames_before, ctx.num_commit_graphs_before);
+ i = ctx.num_commit_graphs_before;
+ g = ctx.r->objects->commit_graph;
while (g) {
- ctx->commit_graph_filenames_before[--i] = xstrdup(g->filename);
+ ctx.commit_graph_filenames_before[--i] = xstrdup(g->filename);
g = g->base_graph;
}
}
- if (ctx->opts)
- replace = ctx->opts->split_flags & COMMIT_GRAPH_SPLIT_REPLACE;
+ if (ctx.opts)
+ replace = ctx.opts->split_flags & COMMIT_GRAPH_SPLIT_REPLACE;
}
- ctx->approx_nr_objects = repo_approximate_object_count(the_repository);
+ ctx.approx_nr_objects = repo_approximate_object_count(the_repository);
- if (ctx->append && ctx->r->objects->commit_graph) {
- struct commit_graph *g = ctx->r->objects->commit_graph;
+ if (ctx.append && ctx.r->objects->commit_graph) {
+ struct commit_graph *g = ctx.r->objects->commit_graph;
for (i = 0; i < g->num_commits; i++) {
struct object_id oid;
oidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_len, i),
the_repository->hash_algo);
- oid_array_append(&ctx->oids, &oid);
+ oid_array_append(&ctx.oids, &oid);
}
}
if (pack_indexes) {
- ctx->order_by_pack = 1;
- if ((res = fill_oids_from_packs(ctx, pack_indexes)))
+ ctx.order_by_pack = 1;
+ if ((res = fill_oids_from_packs(&ctx, pack_indexes)))
goto cleanup;
}
if (commits) {
- if ((res = fill_oids_from_commits(ctx, commits)))
+ if ((res = fill_oids_from_commits(&ctx, commits)))
goto cleanup;
}
if (!pack_indexes && !commits) {
- ctx->order_by_pack = 1;
- fill_oids_from_all_packs(ctx);
+ ctx.order_by_pack = 1;
+ fill_oids_from_all_packs(&ctx);
}
- close_reachable(ctx);
+ close_reachable(&ctx);
- copy_oids_to_commits(ctx);
+ copy_oids_to_commits(&ctx);
- if (ctx->commits.nr >= GRAPH_EDGE_LAST_MASK) {
+ if (ctx.commits.nr >= GRAPH_EDGE_LAST_MASK) {
error(_("too many commits to write graph"));
res = -1;
goto cleanup;
}
- if (!ctx->commits.nr && !replace)
+ if (!ctx.commits.nr && !replace)
goto cleanup;
- if (ctx->split) {
- split_graph_merge_strategy(ctx);
+ if (ctx.split) {
+ split_graph_merge_strategy(&ctx);
if (!replace)
- merge_commit_graphs(ctx);
+ merge_commit_graphs(&ctx);
} else
- ctx->num_commit_graphs_after = 1;
+ ctx.num_commit_graphs_after = 1;
- ctx->trust_generation_numbers = validate_mixed_generation_chain(ctx->r->objects->commit_graph);
+ ctx.trust_generation_numbers = validate_mixed_generation_chain(ctx.r->objects->commit_graph);
- compute_topological_levels(ctx);
- if (ctx->write_generation_data)
- compute_generation_numbers(ctx);
+ compute_topological_levels(&ctx);
+ if (ctx.write_generation_data)
+ compute_generation_numbers(&ctx);
- if (ctx->changed_paths)
- compute_bloom_filters(ctx);
+ if (ctx.changed_paths)
+ compute_bloom_filters(&ctx);
- res = write_commit_graph_file(ctx);
+ res = write_commit_graph_file(&ctx);
- if (ctx->changed_paths)
+ if (ctx.changed_paths)
deinit_bloom_filters();
- if (ctx->split)
- mark_commit_graphs(ctx);
+ if (ctx.split)
+ mark_commit_graphs(&ctx);
- expire_commit_graphs(ctx);
+ expire_commit_graphs(&ctx);
cleanup:
- free(ctx->graph_name);
- free(ctx->base_graph_name);
- free(ctx->commits.list);
- oid_array_clear(&ctx->oids);
+ free(ctx.graph_name);
+ free(ctx.base_graph_name);
+ free(ctx.commits.list);
+ oid_array_clear(&ctx.oids);
clear_topo_level_slab(&topo_levels);
- for (i = 0; i < ctx->num_commit_graphs_before; i++)
- free(ctx->commit_graph_filenames_before[i]);
- free(ctx->commit_graph_filenames_before);
+ for (i = 0; i < ctx.num_commit_graphs_before; i++)
+ free(ctx.commit_graph_filenames_before[i]);
+ free(ctx.commit_graph_filenames_before);
- for (i = 0; i < ctx->num_commit_graphs_after; i++) {
- free(ctx->commit_graph_filenames_after[i]);
- free(ctx->commit_graph_hash_after[i]);
+ for (i = 0; i < ctx.num_commit_graphs_after; i++) {
+ free(ctx.commit_graph_filenames_after[i]);
+ free(ctx.commit_graph_hash_after[i]);
}
- free(ctx->commit_graph_filenames_after);
- free(ctx->commit_graph_hash_after);
-
- free(ctx);
+ free(ctx.commit_graph_filenames_after);
+ free(ctx.commit_graph_hash_after);
return res;
}
--
gitgitgadget
next prev parent reply other threads:[~2025-05-15 13:11 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-15 13:11 [PATCH 00/11] CodeQL-inspired fixes Johannes Schindelin via GitGitGadget
2025-05-15 13:11 ` [PATCH 01/11] commit: simplify code Johannes Schindelin via GitGitGadget
2025-05-15 19:48 ` Jeff King
2025-05-15 20:37 ` Junio C Hamano
2025-05-15 20:49 ` Jeff King
2025-05-15 13:11 ` [PATCH 02/11] fetch: carefully clear local variable's address after use Johannes Schindelin via GitGitGadget
2025-05-15 19:40 ` Jeff King
2025-05-15 13:11 ` Johannes Schindelin via GitGitGadget [this message]
2025-05-15 19:54 ` [PATCH 03/11] commit-graph: avoid malloc'ing a local variable Jeff King
2025-05-15 21:40 ` Junio C Hamano
2025-05-15 13:11 ` [PATCH 04/11] upload-pack: rename `enum` to reflect the operation Johannes Schindelin via GitGitGadget
2025-05-15 19:55 ` Jeff King
2025-05-15 13:11 ` [PATCH 05/11] has_dir_name(): make code more obvious Johannes Schindelin via GitGitGadget
2025-05-15 20:04 ` Jeff King
2025-05-15 13:11 ` [PATCH 06/11] fetch: avoid unnecessary work when there is no current branch Johannes Schindelin via GitGitGadget
2025-05-15 20:11 ` Jeff King
2025-05-15 13:11 ` [PATCH 07/11] Avoid redundant conditions Johannes Schindelin via GitGitGadget
2025-05-15 20:13 ` Jeff King
2025-05-15 13:11 ` [PATCH 08/11] trace2: avoid "futile conditional" Johannes Schindelin via GitGitGadget
2025-05-15 20:16 ` Jeff King
2025-05-15 13:11 ` [PATCH 09/11] commit-graph: avoid using stale stack addresses Johannes Schindelin via GitGitGadget
2025-05-15 20:19 ` Jeff King
2025-05-15 13:11 ` [PATCH 10/11] bundle-uri: avoid using undefined output of `sscanf()` Johannes Schindelin via GitGitGadget
2025-05-15 19:21 ` Junio C Hamano
2025-05-15 20:25 ` Jeff King
2025-05-16 10:11 ` Phillip Wood
2025-05-16 13:40 ` Phillip Wood
2025-05-16 15:42 ` Jeff King
2025-05-19 9:03 ` Phillip Wood
2025-05-22 6:03 ` Jeff King
2025-05-15 13:11 ` [PATCH 11/11] sequencer: stop pretending that an assignment is a condition Johannes Schindelin via GitGitGadget
2025-05-15 18:51 ` Junio C Hamano
2025-05-15 20:26 ` Jeff King
2025-05-16 10:13 ` Phillip Wood
2025-05-15 20:26 ` [PATCH 00/11] CodeQL-inspired fixes Jeff King
2025-05-15 20:58 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5a3a8880a68f8c69c2af39d2e32ebb56eb5fa483.1747314709.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).