Skip to content

Commit 7f3ed75

Browse files
dschogitster
authored andcommitted
commit-graph: avoid malloc'ing a local variable
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c607410 commit 7f3ed75

File tree

1 file changed

+69
-72
lines changed

1 file changed

+69
-72
lines changed

commit-graph.c

Lines changed: 69 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2509,7 +2509,17 @@ int write_commit_graph(struct object_directory *odb,
25092509
const struct commit_graph_opts *opts)
25102510
{
25112511
struct repository *r = the_repository;
2512-
struct write_commit_graph_context *ctx;
2512+
struct write_commit_graph_context ctx = {
2513+
.r = r,
2514+
.odb = odb,
2515+
.append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0,
2516+
.report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0,
2517+
.split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0,
2518+
.opts = opts,
2519+
.total_bloom_filter_data_size = 0,
2520+
.write_generation_data = (get_configured_generation_version(r) == 2),
2521+
.num_generation_data_overflows = 0,
2522+
};
25132523
uint32_t i;
25142524
int res = 0;
25152525
int replace = 0;
@@ -2531,32 +2541,21 @@ int write_commit_graph(struct object_directory *odb,
25312541
return 0;
25322542
}
25332543

2534-
CALLOC_ARRAY(ctx, 1);
2535-
ctx->r = r;
2536-
ctx->odb = odb;
2537-
ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;
2538-
ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;
2539-
ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;
2540-
ctx->opts = opts;
2541-
ctx->total_bloom_filter_data_size = 0;
2542-
ctx->write_generation_data = (get_configured_generation_version(r) == 2);
2543-
ctx->num_generation_data_overflows = 0;
2544-
25452544
bloom_settings.hash_version = r->settings.commit_graph_changed_paths_version;
25462545
bloom_settings.bits_per_entry = git_env_ulong("GIT_TEST_BLOOM_SETTINGS_BITS_PER_ENTRY",
25472546
bloom_settings.bits_per_entry);
25482547
bloom_settings.num_hashes = git_env_ulong("GIT_TEST_BLOOM_SETTINGS_NUM_HASHES",
25492548
bloom_settings.num_hashes);
25502549
bloom_settings.max_changed_paths = git_env_ulong("GIT_TEST_BLOOM_SETTINGS_MAX_CHANGED_PATHS",
25512550
bloom_settings.max_changed_paths);
2552-
ctx->bloom_settings = &bloom_settings;
2551+
ctx.bloom_settings = &bloom_settings;
25532552

25542553
init_topo_level_slab(&topo_levels);
2555-
ctx->topo_levels = &topo_levels;
2554+
ctx.topo_levels = &topo_levels;
25562555

2557-
prepare_commit_graph(ctx->r);
2558-
if (ctx->r->objects->commit_graph) {
2559-
struct commit_graph *g = ctx->r->objects->commit_graph;
2556+
prepare_commit_graph(ctx.r);
2557+
if (ctx.r->objects->commit_graph) {
2558+
struct commit_graph *g = ctx.r->objects->commit_graph;
25602559

25612560
while (g) {
25622561
g->topo_levels = &topo_levels;
@@ -2565,15 +2564,15 @@ int write_commit_graph(struct object_directory *odb,
25652564
}
25662565

25672566
if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
2568-
ctx->changed_paths = 1;
2567+
ctx.changed_paths = 1;
25692568
if (!(flags & COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS)) {
25702569
struct commit_graph *g;
25712570

2572-
g = ctx->r->objects->commit_graph;
2571+
g = ctx.r->objects->commit_graph;
25732572

25742573
/* We have changed-paths already. Keep them in the next graph */
25752574
if (g && g->bloom_filter_settings) {
2576-
ctx->changed_paths = 1;
2575+
ctx.changed_paths = 1;
25772576

25782577
/* don't propagate the hash_version unless unspecified */
25792578
if (bloom_settings.hash_version == -1)
@@ -2586,116 +2585,114 @@ int write_commit_graph(struct object_directory *odb,
25862585

25872586
bloom_settings.hash_version = bloom_settings.hash_version == 2 ? 2 : 1;
25882587

2589-
if (ctx->split) {
2590-
struct commit_graph *g = ctx->r->objects->commit_graph;
2588+
if (ctx.split) {
2589+
struct commit_graph *g = ctx.r->objects->commit_graph;
25912590

25922591
while (g) {
2593-
ctx->num_commit_graphs_before++;
2592+
ctx.num_commit_graphs_before++;
25942593
g = g->base_graph;
25952594
}
25962595

2597-
if (ctx->num_commit_graphs_before) {
2598-
ALLOC_ARRAY(ctx->commit_graph_filenames_before, ctx->num_commit_graphs_before);
2599-
i = ctx->num_commit_graphs_before;
2600-
g = ctx->r->objects->commit_graph;
2596+
if (ctx.num_commit_graphs_before) {
2597+
ALLOC_ARRAY(ctx.commit_graph_filenames_before, ctx.num_commit_graphs_before);
2598+
i = ctx.num_commit_graphs_before;
2599+
g = ctx.r->objects->commit_graph;
26012600

26022601
while (g) {
2603-
ctx->commit_graph_filenames_before[--i] = xstrdup(g->filename);
2602+
ctx.commit_graph_filenames_before[--i] = xstrdup(g->filename);
26042603
g = g->base_graph;
26052604
}
26062605
}
26072606

2608-
if (ctx->opts)
2609-
replace = ctx->opts->split_flags & COMMIT_GRAPH_SPLIT_REPLACE;
2607+
if (ctx.opts)
2608+
replace = ctx.opts->split_flags & COMMIT_GRAPH_SPLIT_REPLACE;
26102609
}
26112610

2612-
ctx->approx_nr_objects = repo_approximate_object_count(the_repository);
2611+
ctx.approx_nr_objects = repo_approximate_object_count(the_repository);
26132612

2614-
if (ctx->append && ctx->r->objects->commit_graph) {
2615-
struct commit_graph *g = ctx->r->objects->commit_graph;
2613+
if (ctx.append && ctx.r->objects->commit_graph) {
2614+
struct commit_graph *g = ctx.r->objects->commit_graph;
26162615
for (i = 0; i < g->num_commits; i++) {
26172616
struct object_id oid;
26182617
oidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_len, i),
26192618
the_repository->hash_algo);
2620-
oid_array_append(&ctx->oids, &oid);
2619+
oid_array_append(&ctx.oids, &oid);
26212620
}
26222621
}
26232622

26242623
if (pack_indexes) {
2625-
ctx->order_by_pack = 1;
2626-
if ((res = fill_oids_from_packs(ctx, pack_indexes)))
2624+
ctx.order_by_pack = 1;
2625+
if ((res = fill_oids_from_packs(&ctx, pack_indexes)))
26272626
goto cleanup;
26282627
}
26292628

26302629
if (commits) {
2631-
if ((res = fill_oids_from_commits(ctx, commits)))
2630+
if ((res = fill_oids_from_commits(&ctx, commits)))
26322631
goto cleanup;
26332632
}
26342633

26352634
if (!pack_indexes && !commits) {
2636-
ctx->order_by_pack = 1;
2637-
fill_oids_from_all_packs(ctx);
2635+
ctx.order_by_pack = 1;
2636+
fill_oids_from_all_packs(&ctx);
26382637
}
26392638

2640-
close_reachable(ctx);
2639+
close_reachable(&ctx);
26412640

2642-
copy_oids_to_commits(ctx);
2641+
copy_oids_to_commits(&ctx);
26432642

2644-
if (ctx->commits.nr >= GRAPH_EDGE_LAST_MASK) {
2643+
if (ctx.commits.nr >= GRAPH_EDGE_LAST_MASK) {
26452644
error(_("too many commits to write graph"));
26462645
res = -1;
26472646
goto cleanup;
26482647
}
26492648

2650-
if (!ctx->commits.nr && !replace)
2649+
if (!ctx.commits.nr && !replace)
26512650
goto cleanup;
26522651

2653-
if (ctx->split) {
2654-
split_graph_merge_strategy(ctx);
2652+
if (ctx.split) {
2653+
split_graph_merge_strategy(&ctx);
26552654

26562655
if (!replace)
2657-
merge_commit_graphs(ctx);
2656+
merge_commit_graphs(&ctx);
26582657
} else
2659-
ctx->num_commit_graphs_after = 1;
2658+
ctx.num_commit_graphs_after = 1;
26602659

2661-
ctx->trust_generation_numbers = validate_mixed_generation_chain(ctx->r->objects->commit_graph);
2660+
ctx.trust_generation_numbers = validate_mixed_generation_chain(ctx.r->objects->commit_graph);
26622661

2663-
compute_topological_levels(ctx);
2664-
if (ctx->write_generation_data)
2665-
compute_generation_numbers(ctx);
2662+
compute_topological_levels(&ctx);
2663+
if (ctx.write_generation_data)
2664+
compute_generation_numbers(&ctx);
26662665

2667-
if (ctx->changed_paths)
2668-
compute_bloom_filters(ctx);
2666+
if (ctx.changed_paths)
2667+
compute_bloom_filters(&ctx);
26692668

2670-
res = write_commit_graph_file(ctx);
2669+
res = write_commit_graph_file(&ctx);
26712670

2672-
if (ctx->changed_paths)
2671+
if (ctx.changed_paths)
26732672
deinit_bloom_filters();
26742673

2675-
if (ctx->split)
2676-
mark_commit_graphs(ctx);
2674+
if (ctx.split)
2675+
mark_commit_graphs(&ctx);
26772676

2678-
expire_commit_graphs(ctx);
2677+
expire_commit_graphs(&ctx);
26792678

26802679
cleanup:
2681-
free(ctx->graph_name);
2682-
free(ctx->base_graph_name);
2683-
free(ctx->commits.list);
2684-
oid_array_clear(&ctx->oids);
2680+
free(ctx.graph_name);
2681+
free(ctx.base_graph_name);
2682+
free(ctx.commits.list);
2683+
oid_array_clear(&ctx.oids);
26852684
clear_topo_level_slab(&topo_levels);
26862685

2687-
for (i = 0; i < ctx->num_commit_graphs_before; i++)
2688-
free(ctx->commit_graph_filenames_before[i]);
2689-
free(ctx->commit_graph_filenames_before);
2686+
for (i = 0; i < ctx.num_commit_graphs_before; i++)
2687+
free(ctx.commit_graph_filenames_before[i]);
2688+
free(ctx.commit_graph_filenames_before);
26902689

2691-
for (i = 0; i < ctx->num_commit_graphs_after; i++) {
2692-
free(ctx->commit_graph_filenames_after[i]);
2693-
free(ctx->commit_graph_hash_after[i]);
2690+
for (i = 0; i < ctx.num_commit_graphs_after; i++) {
2691+
free(ctx.commit_graph_filenames_after[i]);
2692+
free(ctx.commit_graph_hash_after[i]);
26942693
}
2695-
free(ctx->commit_graph_filenames_after);
2696-
free(ctx->commit_graph_hash_after);
2697-
2698-
free(ctx);
2694+
free(ctx.commit_graph_filenames_after);
2695+
free(ctx.commit_graph_hash_after);
26992696

27002697
return res;
27012698
}

0 commit comments

Comments
 (0)