git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, me@ttaylorr.com,
	Derrick Stolee <stolee@gmail.com>,
	Derrick Stolee <stolee@gmail.com>
Subject: [PATCH v2 6/6] midx-write: simplify error cases
Date: Sat, 30 Aug 2025 21:23:27 +0000	[thread overview]
Message-ID: <7be25cf5349c389cf2887ab5b852779fc364bd7e.1756589007.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1965.v2.git.1756589007.gitgitgadget@gmail.com>

From: Derrick Stolee <stolee@gmail.com>

The write_midx_internal() method uses gotos to jump to a cleanup section to
clear memory before returning 'result'. Since these jumps are more common
for error conditions, initialize 'result' to -1 and then only set it to 0
before returning with success. There are a couple places where we return
with success via a jump.

This has the added benefit that the method now returns -1 on error instead
of an inconsistent 1 or -1.

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

diff --git a/midx-write.c b/midx-write.c
index 14a0947c46..047ffdcdbf 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -1046,7 +1046,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 	int bitmapped_packs_concat_len = 0;
 	int pack_name_concat_len = 0;
 	int dropped_packs = 0;
-	int result = 0;
+	int result = -1;
 	const char **keep_hashes = NULL;
 	struct chunkfile *cf;
 
@@ -1099,14 +1099,12 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 				error(_("could not load reverse index for MIDX %s"),
 				      hash_to_hex_algop(get_midx_checksum(m),
 							m->repo->hash_algo));
-				result = 1;
 				goto cleanup;
 			}
 			ctx.num_multi_pack_indexes_before++;
 			m = m->base_midx;
 		}
 	} else if (ctx.m && fill_packs_from_midx(&ctx)) {
-		result = 1;
 		goto cleanup;
 	}
 
@@ -1142,12 +1140,16 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 			 */
 			if (!want_bitmap)
 				clear_midx_files_ext(object_dir, "bitmap", NULL);
+
+			result = 0;
 			goto cleanup;
 		}
 	}
 
-	if (ctx.incremental && !ctx.nr)
+	if (ctx.incremental && !ctx.nr) {
+		result = 0;
 		goto cleanup; /* nothing to do */
+	}
 
 	if (preferred_pack_name) {
 		ctx.preferred_pack_idx = NO_PREFERRED_PACK;
@@ -1221,7 +1223,6 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 		if (!preferred->num_objects) {
 			error(_("cannot select preferred pack %s with no objects"),
 			      preferred->pack_name);
-			result = 1;
 			goto cleanup;
 		}
 	}
@@ -1260,10 +1261,8 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 			}
 		}
 
-		if (missing_drops) {
-			result = 1;
+		if (missing_drops)
 			goto cleanup;
-		}
 	}
 
 	/*
@@ -1309,7 +1308,6 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 
 	if (ctx.nr - dropped_packs == 0) {
 		error(_("no pack files to index."));
-		result = 1;
 		goto cleanup;
 	}
 
@@ -1329,14 +1327,12 @@ 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"));
-			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));
-			result = -1;
 			goto cleanup;
 		}
 
@@ -1414,7 +1410,6 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 				      midx_hash, &pdata, commits, commits_nr,
 				      flags) < 0) {
 			error(_("could not write multi-pack bitmap"));
-			result = 1;
 			clear_packing_data(&pdata);
 			free(commits);
 			goto cleanup;
@@ -1440,21 +1435,17 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 
 		if (!chainf) {
 			error_errno(_("unable to open multi-pack-index chain file"));
-			result = -1;
 			goto cleanup;
 		}
 
-		if (link_midx_to_chain(ctx.base_midx) < 0) {
-			result = -1;
+		if (link_midx_to_chain(ctx.base_midx) < 0)
 			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"));
-			result = -1;
 			goto cleanup;
 		}
 
@@ -1487,6 +1478,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
 	clear_midx_files(r, object_dir, keep_hashes,
 			 ctx.num_multi_pack_indexes_before + 1,
 			 ctx.incremental);
+	result = 0;
 
 cleanup:
 	for (size_t i = 0; i < ctx.nr; i++) {
-- 
gitgitgadget

  parent reply	other threads:[~2025-08-30 21:23 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-28 17:39 [PATCH 0/5] midx-write: fix segfault and do several cleanups Derrick Stolee via GitGitGadget
2025-08-28 17:39 ` [PATCH 1/5] midx-write: only load initialized packs Derrick Stolee via GitGitGadget
2025-08-28 20:19   ` Junio C Hamano
2025-08-29  1:20   ` Taylor Blau
2025-08-30 14:33     ` Derrick Stolee
2025-08-28 17:39 ` [PATCH 2/5] midx-write: put failing response value back Derrick Stolee via GitGitGadget
2025-08-28 20:45   ` Junio C Hamano
2025-08-29  1:26     ` Taylor Blau
2025-08-28 17:39 ` [PATCH 3/5] midx-write: use cleanup when incremental midx fails Derrick Stolee via GitGitGadget
2025-08-28 20:51   ` Junio C Hamano
2025-08-29  1:29     ` Taylor Blau
2025-08-30 14:44       ` Derrick Stolee
2025-08-28 17:39 ` [PATCH 4/5] midx-write: use uint32_t for preferred_pack_idx Derrick Stolee via GitGitGadget
2025-08-28 20:58   ` Junio C Hamano
2025-08-29  1:35   ` Taylor Blau
2025-08-28 17:39 ` [PATCH 5/5] midx-write: reenable signed comparison errors Derrick Stolee via GitGitGadget
2025-08-28 21:01   ` Junio C Hamano
2025-08-29  1:35     ` Taylor Blau
2025-08-29  1:36 ` [PATCH 0/5] midx-write: fix segfault and do several cleanups Taylor Blau
2025-08-30 21:23 ` [PATCH v2 0/6] " Derrick Stolee via GitGitGadget
2025-08-30 21:23   ` [PATCH v2 1/6] midx-write: only load initialized packs Derrick Stolee via GitGitGadget
2025-09-03 10:14     ` Patrick Steinhardt
2025-09-05 18:58       ` Derrick Stolee
2025-08-30 21:23   ` [PATCH v2 2/6] midx-write: put failing response value back Derrick Stolee via GitGitGadget
2025-09-03 10:15     ` Patrick Steinhardt
2025-09-05 19:03       ` Derrick Stolee
2025-08-30 21:23   ` [PATCH v2 3/6] midx-write: use cleanup when incremental midx fails Derrick Stolee via GitGitGadget
2025-09-03 10:15     ` Patrick Steinhardt
2025-08-30 21:23   ` [PATCH v2 4/6] midx-write: use uint32_t for preferred_pack_idx Derrick Stolee via GitGitGadget
2025-09-03 10:15     ` Patrick Steinhardt
2025-09-05 19:05       ` Derrick Stolee
2025-08-30 21:23   ` [PATCH v2 5/6] midx-write: reenable signed comparison errors Derrick Stolee via GitGitGadget
2025-09-03 10:15     ` Patrick Steinhardt
2025-08-30 21:23   ` Derrick Stolee via GitGitGadget [this message]
2025-09-03 10:15     ` [PATCH v2 6/6] midx-write: simplify error cases Patrick Steinhardt
2025-09-03 18:43       ` Junio C Hamano
2025-09-05 19:26   ` [PATCH v3 0/6] midx-write: fix segfault and do several cleanups Derrick Stolee via GitGitGadget
2025-09-05 19:26     ` [PATCH v3 1/6] midx-write: only load initialized packs Derrick Stolee via GitGitGadget
2025-09-05 19:26     ` [PATCH v3 2/6] midx-write: put failing response value back Derrick Stolee via GitGitGadget
2025-09-05 19:26     ` [PATCH v3 3/6] midx-write: use cleanup when incremental midx fails Derrick Stolee via GitGitGadget
2025-09-05 19:26     ` [PATCH v3 4/6] midx-write: use uint32_t for preferred_pack_idx Derrick Stolee via GitGitGadget
2025-09-05 19:26     ` [PATCH v3 5/6] midx-write: reenable signed comparison errors Derrick Stolee via GitGitGadget
2025-09-05 19:26     ` [PATCH v3 6/6] midx-write: simplify error cases Derrick Stolee via GitGitGadget
2025-09-05 19:38     ` [PATCH v3 0/6] midx-write: fix segfault and do several cleanups Junio C Hamano
2025-09-05 19:57       ` Derrick Stolee
2025-09-11 23:13         ` Taylor Blau
2025-09-11 23:44           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7be25cf5349c389cf2887ab5b852779fc364bd7e.1756589007.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).