[PATCH v3 3/6] midx-write: use cleanup when incremental midx fails

From: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date: Sep 5, 2025, 19:26 UTC
Message-ID: <414ae51024ef04370180fd550b4cd51980cc7133.1757100378.git.gitgitgadget@gmail.com>
In-Reply-To: <pull.1965.v3.git.1757100378.gitgitgadget@gmail.com>

From: Derrick Stolee <stolee@gmail.com>

The incremental mode of writing a multi-pack-index has a few extra conditions that could lead to failure, but these are currently short-ciruiting with 'return -1' instead of setting the method's 'result' variable and going to the cleanup tag.

Replace these returns with gotos to avoid memory issues when exiting early due to error conditions.

Unfortunately, these error conditions are difficult to reproduce with test cases, which is perhaps one reason why the memory loss was not caught by existing test cases in memory tracking modes.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 midx-write.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/midx-write.c b/midx-write.c
index 0f1d5653ab..cb0211289d 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -1327,13 +1327,15 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 		incr = mks_tempfile_m(midx_name.buf, 0444);
 		if (!incr) {
 			error(_("unable to create temporary MIDX layer"));
-			return -1;
+			result = -1;
+			goto cleanup;
 		}
 
 		if (adjust_shared_perm(r, get_tempfile_path(incr))) {
 			error(_("unable to adjust shared permissions for '%s'"),
 			      get_tempfile_path(incr));
-			return -1;
+			result = -1;
+			goto cleanup;
 		}
 
 		f = hashfd(r->hash_algo, get_tempfile_fd(incr),
@@ -1433,18 +1435,22 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 
 		if (!chainf) {
 			error_errno(_("unable to open multi-pack-index chain file"));
-			return -1;
+			result = -1;
+			goto cleanup;
 		}
 
-		if (link_midx_to_chain(ctx.base_midx) < 0)
-			return -1;
+		if (link_midx_to_chain(ctx.base_midx) < 0) {
+			result = -1;
+			goto cleanup;
+		}
 
 		get_split_midx_filename_ext(r->hash_algo, &final_midx_name,
 					    object_dir, midx_hash, MIDX_EXT_MIDX);
 
 		if (rename_tempfile(&incr, final_midx_name.buf) < 0) {
 			error_errno(_("unable to rename new multi-pack-index layer"));
-			return -1;
+			result = -1;
+			goto cleanup;
 		}
 
 		strbuf_release(&final_midx_name);

-- 
gitgitgadget