@matheusbsbs / matheus-bagliano / commits / 7f3ed75ff5

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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed May 15, 2025 at 13:11 UTC 7f3ed75ff551e2ca4f8eb0242784e7aacbb14fb3

1 file changed +69 -72

commit-graph.c

+69 -72

@@ -2509,7 +2509,17 @@ int write_commit_graph(struct object_directory *odb,

2509 const struct commit_graph_opts *opts)

2510 {

2511 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 + };

2523 uint32_t i;

2524 int res = 0;

2525 int replace = 0;

@@ -2531,17 +2541,6 @@ int write_commit_graph(struct object_directory *odb,

2541 return 0;

2542 }

2543

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 -

2544 bloom_settings.hash_version = r->settings.commit_graph_changed_paths_version;

2545 bloom_settings.bits_per_entry = git_env_ulong("GIT_TEST_BLOOM_SETTINGS_BITS_PER_ENTRY",

2546 bloom_settings.bits_per_entry);

@@ -2549,14 +2548,14 @@ int write_commit_graph(struct object_directory *odb,

2548 bloom_settings.num_hashes);

2549 bloom_settings.max_changed_paths = git_env_ulong("GIT_TEST_BLOOM_SETTINGS_MAX_CHANGED_PATHS",

2550 bloom_settings.max_changed_paths);

2552 - ctx->bloom_settings = &bloom_settings;

2551 + ctx.bloom_settings = &bloom_settings;

2552

2553 init_topo_level_slab(&topo_levels);

2555 - ctx->topo_levels = &topo_levels;

2554 + ctx.topo_levels = &topo_levels;

2555

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;

2559

2560 while (g) {

2561 g->topo_levels = &topo_levels;

@@ -2565,15 +2564,15 @@ int write_commit_graph(struct object_directory *odb,

2564 }

2565

2566 if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)

2568 - ctx->changed_paths = 1;

2567 + ctx.changed_paths = 1;

2568 if (!(flags & COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS)) {

2569 struct commit_graph *g;

2570

2572 - g = ctx->r->objects->commit_graph;

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

2572

2573 /* We have changed-paths already. Keep them in the next graph */

2574 if (g && g->bloom_filter_settings) {

2576 - ctx->changed_paths = 1;

2575 + ctx.changed_paths = 1;

2576

2577 /* don't propagate the hash_version unless unspecified */

2578 if (bloom_settings.hash_version == -1)

@@ -2586,116 +2585,114 @@ int write_commit_graph(struct object_directory *odb,

2585

2586 bloom_settings.hash_version = bloom_settings.hash_version == 2 ? 2 : 1;

2587

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;

2590

2591 while (g) {

2593 - ctx->num_commit_graphs_before++;

2592 + ctx.num_commit_graphs_before++;

2593 g = g->base_graph;

2594 }

2595

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;

2600

2601 while (g) {

2603 - ctx->commit_graph_filenames_before[--i] = xstrdup(g->filename);

2602 + ctx.commit_graph_filenames_before[--i] = xstrdup(g->filename);

2603 g = g->base_graph;

2604 }

2605 }

2606

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;

2609 }

2610

2612 - ctx->approx_nr_objects = repo_approximate_object_count(the_repository);

2611 + ctx.approx_nr_objects = repo_approximate_object_count(the_repository);

2612

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;

2615 for (i = 0; i < g->num_commits; i++) {

2616 struct object_id oid;

2617 oidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_len, i),

2618 the_repository->hash_algo);

2620 - oid_array_append(&ctx->oids, &oid);

2619 + oid_array_append(&ctx.oids, &oid);

2620 }

2621 }

2622

2623 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)))

2626 goto cleanup;

2627 }

2628

2629 if (commits) {

2631 - if ((res = fill_oids_from_commits(ctx, commits)))

2630 + if ((res = fill_oids_from_commits(&ctx, commits)))

2631 goto cleanup;

2632 }

2633

2634 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);

2637 }

2638

2640 - close_reachable(ctx);

2639 + close_reachable(&ctx);

2640

2642 - copy_oids_to_commits(ctx);

2641 + copy_oids_to_commits(&ctx);

2642

2644 - if (ctx->commits.nr >= GRAPH_EDGE_LAST_MASK) {

2643 + if (ctx.commits.nr >= GRAPH_EDGE_LAST_MASK) {

2644 error(_("too many commits to write graph"));

2645 res = -1;

2646 goto cleanup;

2647 }

2648

2650 - if (!ctx->commits.nr && !replace)

2649 + if (!ctx.commits.nr && !replace)

2650 goto cleanup;

2651

2653 - if (ctx->split) {

2654 - split_graph_merge_strategy(ctx);

2652 + if (ctx.split) {

2653 + split_graph_merge_strategy(&ctx);

2654

2655 if (!replace)

2657 - merge_commit_graphs(ctx);

2656 + merge_commit_graphs(&ctx);

2657 } else

2659 - ctx->num_commit_graphs_after = 1;

2658 + ctx.num_commit_graphs_after = 1;

2659

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);

2661

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);

2665

2667 - if (ctx->changed_paths)

2668 - compute_bloom_filters(ctx);

2666 + if (ctx.changed_paths)

2667 + compute_bloom_filters(&ctx);

2668

2670 - res = write_commit_graph_file(ctx);

2669 + res = write_commit_graph_file(&ctx);

2670

2672 - if (ctx->changed_paths)

2671 + if (ctx.changed_paths)

2672 deinit_bloom_filters();

2673

2675 - if (ctx->split)

2676 - mark_commit_graphs(ctx);

2674 + if (ctx.split)

2675 + mark_commit_graphs(&ctx);

2676

2678 - expire_commit_graphs(ctx);

2677 + expire_commit_graphs(&ctx);

2678

2679 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);

2684 clear_topo_level_slab(&topo_levels);

2685

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);

2689

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]);

2693 }

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);

2696

2697 return res;

2698 }