git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/8] packfile: have `for_each_file_in_pack_dir()` return error codes

From
Patrick Steinhardt <ps@pks.im>
Date
May 19, 2021, 19:13 UTC
Message-ID
<64a1bcf2c11d180542e06f67c451f9f90019dea4.1621451532.git.ps@pks.im>
In-Reply-To
<cover.1621451532.git.ps@pks.im>

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

-- 
2.31.1
Previous: Elijah NewrenNext: Patrick Steinhardt
Message 10 of 22 in “Speed up connectivity checks via quarantine dir”
  1. 0/8 Speed up connectivity checks via quarantine dirPatrick Steinhardt, May 19, 2021
  2. 1/8 perf: fix when running with TEST_OUTPUT_DIRECTORYPatrick Steinhardt, May 19, 2021
  3. Chris TorekMay 20, 2021
  4. 2/8 p5400: add perf tests for git-receive-pack(1)Patrick Steinhardt, May 19, 2021
  5. Chris TorekMay 20, 2021
  6. Jeff KingMay 20, 2021
  7. SZEDER GáborMay 21, 2021
  8. 3/8 tmp-objdir: expose function to retrieve pathPatrick Steinhardt, May 19, 2021
  9. Elijah NewrenMay 20, 2021
  10. 4/8 packfile: have `for_each_file_in_pack_dir()` return error codesPatrick Steinhardt, May 19, 2021
  11. 5/8 object-file: allow reading loose objects without reading their contentsPatrick Steinhardt, May 19, 2021
  12. 6/8 connected: implement connectivity check via temporary object dirsPatrick Steinhardt, May 19, 2021
  13. 7/8 receive-pack: skip connectivity checks on delete-only commandsPatrick Steinhardt, May 19, 2021
  14. Felipe ContrerasMay 21, 2021
  15. Jeff KingMay 27, 2021
  16. 8/8 receive-pack: check connectivity via quarantined objectsPatrick Steinhardt, May 19, 2021
  17. Chris TorekMay 20, 2021
  18. Jeff KingMay 20, 2021
  19. Junio C HamanoMay 20, 2021
  20. Jeff KingMay 21, 2021
  21. Patrick SteinhardtMay 21, 2021
  22. Ævar Arnfjörð BjarmasonMay 21, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.