git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Subject: [PATCH 4/8] packfile: have `for_each_file_in_pack_dir()` return error codes
Date: Wed, 19 May 2021 21:13:35 +0200	[thread overview]
Message-ID: <64a1bcf2c11d180542e06f67c451f9f90019dea4.1621451532.git.ps@pks.im> (raw)
In-Reply-To: <cover.1621451532.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 6380 bytes --]

The function `for_each_file_in_pack_dir()` doesn't ever return error
codes right now: any errors it hits are simply swallowed. A new user
we're about to add would like to know what's going on though such that
it can decide whether the call was successful or not.

Refactor the function to return an error code, and adapt the signature
of the callback function to also return an error code. This enables the
callback to abort iteration and have its error code passed through.
Existing callers are adapted to this change, but keep ignoring errors.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 midx.c     | 22 +++++++++++++---------
 packfile.c | 26 ++++++++++++++++----------
 packfile.h | 10 +++++-----
 3 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/midx.c b/midx.c
index 21d6a05e88..f87a3d36ab 100644
--- a/midx.c
+++ b/midx.c
@@ -470,15 +470,15 @@ struct write_midx_context {
 	int preferred_pack_idx;
 };
 
-static void add_pack_to_midx(const char *full_path, size_t full_path_len,
-			     const char *file_name, void *data)
+static int add_pack_to_midx(const char *full_path, size_t full_path_len,
+			    const char *file_name, void *data)
 {
 	struct write_midx_context *ctx = data;
 
 	if (ends_with(file_name, ".idx")) {
 		display_progress(ctx->progress, ++ctx->pack_paths_checked);
 		if (ctx->m && midx_contains_pack(ctx->m, file_name))
-			return;
+			return 0;
 
 		ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc);
 
@@ -489,7 +489,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
 		if (!ctx->info[ctx->nr].p) {
 			warning(_("failed to add packfile '%s'"),
 				full_path);
-			return;
+			return 0;
 		}
 
 		if (open_pack_index(ctx->info[ctx->nr].p)) {
@@ -497,7 +497,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
 				full_path);
 			close_pack(ctx->info[ctx->nr].p);
 			FREE_AND_NULL(ctx->info[ctx->nr].p);
-			return;
+			return 0;
 		}
 
 		ctx->info[ctx->nr].pack_name = xstrdup(file_name);
@@ -505,6 +505,8 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
 		ctx->info[ctx->nr].expired = 0;
 		ctx->nr++;
 	}
+
+	return 0;
 }
 
 struct pack_midx_entry {
@@ -1110,19 +1112,21 @@ struct clear_midx_data {
 	const char *ext;
 };
 
-static void clear_midx_file_ext(const char *full_path, size_t full_path_len,
-				const char *file_name, void *_data)
+static int clear_midx_file_ext(const char *full_path, size_t full_path_len,
+			const char *file_name, void *_data)
 {
 	struct clear_midx_data *data = _data;
 
 	if (!(starts_with(file_name, "multi-pack-index-") &&
 	      ends_with(file_name, data->ext)))
-		return;
+		return 0;
 	if (data->keep && !strcmp(data->keep, file_name))
-		return;
+		return 0;
 
 	if (unlink(full_path))
 		die_errno(_("failed to remove %s"), full_path);
+
+	return 0;
 }
 
 static void clear_midx_files_ext(struct repository *r, const char *ext,
diff --git a/packfile.c b/packfile.c
index b79cbc8cd4..f488a214cf 100644
--- a/packfile.c
+++ b/packfile.c
@@ -792,14 +792,15 @@ static void report_pack_garbage(struct string_list *list)
 	report_helper(list, seen_bits, first, list->nr);
 }
 
-void for_each_file_in_pack_dir(const char *objdir,
-			       each_file_in_pack_dir_fn fn,
-			       void *data)
+int for_each_file_in_pack_dir(const char *objdir,
+			      each_file_in_pack_dir_fn fn,
+			      void *data)
 {
 	struct strbuf path = STRBUF_INIT;
 	size_t dirnamelen;
 	DIR *dir;
 	struct dirent *de;
+	int ret = 0;
 
 	strbuf_addstr(&path, objdir);
 	strbuf_addstr(&path, "/pack");
@@ -809,7 +810,7 @@ void for_each_file_in_pack_dir(const char *objdir,
 			error_errno("unable to open object pack directory: %s",
 				    path.buf);
 		strbuf_release(&path);
-		return;
+		return -1;
 	}
 	strbuf_addch(&path, '/');
 	dirnamelen = path.len;
@@ -820,11 +821,14 @@ void for_each_file_in_pack_dir(const char *objdir,
 		strbuf_setlen(&path, dirnamelen);
 		strbuf_addstr(&path, de->d_name);
 
-		fn(path.buf, path.len, de->d_name, data);
+		ret = fn(path.buf, path.len, de->d_name, data);
+		if (ret)
+			break;
 	}
 
 	closedir(dir);
 	strbuf_release(&path);
+	return ret;
 }
 
 struct prepare_pack_data {
@@ -834,8 +838,8 @@ struct prepare_pack_data {
 	struct multi_pack_index *m;
 };
 
-static void prepare_pack(const char *full_name, size_t full_name_len,
-			 const char *file_name, void *_data)
+static int prepare_pack(const char *full_name, size_t full_name_len,
+			const char *file_name, void *_data)
 {
 	struct prepare_pack_data *data = (struct prepare_pack_data *)_data;
 	struct packed_git *p;
@@ -858,13 +862,13 @@ static void prepare_pack(const char *full_name, size_t full_name_len,
 	}
 
 	if (!report_garbage)
-		return;
+		return 0;
 
 	if (!strcmp(file_name, "multi-pack-index"))
-		return;
+		return 0;
 	if (starts_with(file_name, "multi-pack-index") &&
 	    ends_with(file_name, ".rev"))
-		return;
+		return 0;
 	if (ends_with(file_name, ".idx") ||
 	    ends_with(file_name, ".rev") ||
 	    ends_with(file_name, ".pack") ||
@@ -874,6 +878,8 @@ static void prepare_pack(const char *full_name, size_t full_name_len,
 		string_list_append(data->garbage, full_name);
 	else
 		report_garbage(PACKDIR_FILE_GARBAGE, full_name);
+
+	return 0;
 }
 
 static void prepare_packed_git_one(struct repository *r, char *objdir, int local)
diff --git a/packfile.h b/packfile.h
index 3ae117a8ae..eac930cd27 100644
--- a/packfile.h
+++ b/packfile.h
@@ -39,11 +39,11 @@ const char *pack_basename(struct packed_git *p);
 
 struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
 
-typedef void each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len,
-				      const char *file_pach, void *data);
-void for_each_file_in_pack_dir(const char *objdir,
-			       each_file_in_pack_dir_fn fn,
-			       void *data);
+typedef int each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len,
+				     const char *file_pach, void *data);
+int for_each_file_in_pack_dir(const char *objdir,
+			      each_file_in_pack_dir_fn fn,
+			      void *data);
 
 /* A hook to report invalid files in pack directory */
 #define PACKDIR_FILE_PACK 1
-- 
@@ -39,11 +39,11 @@ const char *pack_basename(struct packed_git *p);
 
 struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
 
-typedef void each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len,
-				      const char *file_pach, void *data);
-void for_each_file_in_pack_dir(const char *objdir,
-			       each_file_in_pack_dir_fn fn,
-			       void *data);
+typedef int each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len,
+				     const char *file_pach, void *data);
+int for_each_file_in_pack_dir(const char *objdir,
+			      each_file_in_pack_dir_fn fn,
+			      void *data);
 
 /* A hook to report invalid files in pack directory */
 #define PACKDIR_FILE_PACK 1
-- 
2.31.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2021-05-19 19:13 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-19 19:13 [PATCH 0/8] Speed up connectivity checks via quarantine dir Patrick Steinhardt
2021-05-19 19:13 ` [PATCH 1/8] perf: fix when running with TEST_OUTPUT_DIRECTORY Patrick Steinhardt
2021-05-20  2:03   ` Chris Torek
2021-05-19 19:13 ` [PATCH 2/8] p5400: add perf tests for git-receive-pack(1) Patrick Steinhardt
2021-05-20  2:09   ` Chris Torek
2021-05-20 17:04   ` Jeff King
2021-05-21 15:03   ` SZEDER Gábor
2021-05-19 19:13 ` [PATCH 3/8] tmp-objdir: expose function to retrieve path Patrick Steinhardt
2021-05-20  0:16   ` Elijah Newren
2021-05-19 19:13 ` Patrick Steinhardt [this message]
2021-05-19 19:13 ` [PATCH 5/8] object-file: allow reading loose objects without reading their contents Patrick Steinhardt
2021-05-19 19:13 ` [PATCH 6/8] connected: implement connectivity check via temporary object dirs Patrick Steinhardt
2021-05-19 19:13 ` [PATCH 7/8] receive-pack: skip connectivity checks on delete-only commands Patrick Steinhardt
2021-05-21 18:53   ` Felipe Contreras
2021-05-27 14:38     ` Jeff King
2021-05-19 19:13 ` [PATCH 8/8] receive-pack: check connectivity via quarantined objects Patrick Steinhardt
2021-05-20  2:19 ` [PATCH 0/8] Speed up connectivity checks via quarantine dir Chris Torek
2021-05-20 16:50 ` Jeff King
2021-05-20 21:45   ` Junio C Hamano
2021-05-21  9:30     ` Jeff King
2021-05-21  9:42   ` Patrick Steinhardt
2021-05-21 11:20   ` Ævar Arnfjörð Bjarmason

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=64a1bcf2c11d180542e06f67c451f9f90019dea4.1621451532.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).