Thread (8 messages) flat view 8 messages, 5 authors, 2023-08-11

leak in jt/path-filter-fix, was Re: What's cooking in git.git (Aug 2023, #01; Wed, 2)

From: Jeff King <hidden>
Date: 2023-08-08 20:18:42
Subsystem: the rest · Maintainer: Linus Torvalds

On Wed, Aug 02, 2023 at 11:10:34AM -0700, Junio C Hamano wrote:
* jt/path-filter-fix (2023-08-01) 7 commits
 - commit-graph: new filter ver. that fixes murmur3
 - repo-settings: introduce commitgraph.changedPathsVersion
 - t4216: test changed path filters with high bit paths
 - t/helper/test-read-graph: implement `bloom-filters` mode
 - bloom.h: make `load_bloom_filter_from_graph()` public
 - t/helper/test-read-graph.c: extract `dump_graph_info()`
 - gitformat-commit-graph: describe version 2 of BDAT

 The Bloom filter used for path limited history traversal was broken
 on systems whose "char" is unsigned; update the implementation and
 bump the format version to 2.

 Still under discussion.
 cf. [ref]
 source: [ref]
Since this hit 'next', Coverity complained about a small leak. Fixed by
the patch below.

-- >8 --
Subject: [PATCH] commit-graph: fix small leak with invalid changedPathsVersion

When writing a commit graph, we sanity-check the value of
commitgraph.changedPathsVersion and return early if it's invalid. But we
only do so after allocating the write_commit_graph_context, meaing we
leak it. We could "goto cleanup" to fix this, but instead let's push
this check to the top of the function with the other early checks which
return before doing any work.

Signed-off-by: Jeff King <redacted>
---
 commit-graph.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 38edb12669..ccc21c6708 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2372,6 +2372,12 @@ int write_commit_graph(struct object_directory *odb,
 	}
 	if (!commit_graph_compatible(r))
 		return 0;
+	if (r->settings.commit_graph_changed_paths_version < -1
+	    || r->settings.commit_graph_changed_paths_version > 2) {
+		warning(_("attempting to write a commit-graph, but 'commitgraph.changedPathsVersion' (%d) is not supported"),
+			r->settings.commit_graph_changed_paths_version);
+		return 0;
+	}
 
 	CALLOC_ARRAY(ctx, 1);
 	ctx->r = r;
@@ -2384,12 +2390,6 @@ int write_commit_graph(struct object_directory *odb,
 	ctx->write_generation_data = (get_configured_generation_version(r) == 2);
 	ctx->num_generation_data_overflows = 0;
 
-	if (r->settings.commit_graph_changed_paths_version < -1
-	    || r->settings.commit_graph_changed_paths_version > 2) {
-		warning(_("attempting to write a commit-graph, but 'commitgraph.changedPathsVersion' (%d) is not supported"),
-			r->settings.commit_graph_changed_paths_version);
-		return 0;
-	}
 	bloom_settings.hash_version = r->settings.commit_graph_changed_paths_version == 2
 		? 2 : 1;
 	bloom_settings.bits_per_entry = git_env_ulong("GIT_TEST_BLOOM_SETTINGS_BITS_PER_ENTRY",
-- 
2.42.0.rc0.376.g66bfc4f195
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help