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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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. <20230801185232.1457172-1-jonathantanmy@xxxxxxxxxx>
>  source: <cover.1690912539.git.jonathantanmy@xxxxxxxxxx>

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 <peff@xxxxxxxx>
---
 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux