packfile: introduce macro to iterate through packs

pks-t · gitster · commit 55ee631b0976 · 2025-10-07T10:57:35.000-07:00
We have a bunch of different sites that want to iterate through all
packs of a given `struct packfile_store`. This pattern is somewhat
verbose and repetitive, which makes it somewhat cumbersome.

Introduce a new macro `packfile_store_for_each_pack()` that removes some
of the boilerplate.

Signed-off-by: Patrick Steinhardt &lt;ps@pks.im&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
@@ -855,7 +855,7 @@ static void batch_each_object(struct batch_options *opt,
 		struct packfile_store *packs = the_repository->objects->packfiles;
 		struct packed_git *pack;
 
-		for (pack = packfile_store_get_all_packs(packs); pack; pack = pack->next) {
+		packfile_store_for_each_pack(packs, pack) {
 			if (bitmap_index_contains_pack(bitmap, pack) ||
 			    open_pack_index(pack))
 				continue;
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
@@ -130,7 +130,7 @@ int cmd_count_objects(int argc,
 		struct strbuf pack_buf = STRBUF_INIT;
 		struct strbuf garbage_buf = STRBUF_INIT;
 
-		for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+		packfile_store_for_each_pack(packs, p) {
 			if (!p->pack_local)
 				continue;
 			if (open_pack_index(p))
diff --git a/builtin/fsck.c b/builtin/fsck.c
@@ -869,18 +869,19 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)
 {
 	struct packfile_store *packs = r->objects->packfiles;
 	struct progress *progress = NULL;
+	struct packed_git *p;
 	uint32_t pack_count = 0;
 	int res = 0;
 
 	if (show_progress) {
-		for (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next)
+		packfile_store_for_each_pack(packs, p)
 			pack_count++;
 		progress = start_delayed_progress(the_repository,
 						  "Verifying reverse pack-indexes", pack_count);
 		pack_count = 0;
 	}
 
-	for (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		int load_error = load_pack_revindex_from_disk(p);
 
 		if (load_error < 0) {
@@ -1012,8 +1013,7 @@ int cmd_fsck(int argc,
 			struct progress *progress = NULL;
 
 			if (show_progress) {
-				for (p = packfile_store_get_all_packs(packs); p;
-				     p = p->next) {
+				packfile_store_for_each_pack(packs, p) {
 					if (open_pack_index(p))
 						continue;
 					total += p->num_objects;
@@ -1022,8 +1022,8 @@ int cmd_fsck(int argc,
 				progress = start_progress(the_repository,
 							  _("Checking objects"), total);
 			}
-			for (p = packfile_store_get_all_packs(packs); p;
-			     p = p->next) {
+
+			packfile_store_for_each_pack(packs, p) {
 				/* verify gives error messages itself */
 				if (verify_pack(the_repository,
 						p, fsck_obj_buffer,
diff --git a/builtin/gc.c b/builtin/gc.c
@@ -490,7 +490,7 @@ static struct packed_git *find_base_packs(struct string_list *packs,
 	struct packfile_store *packfiles = the_repository->objects->packfiles;
 	struct packed_git *p, *base = NULL;
 
-	for (p = packfile_store_get_all_packs(packfiles); p; p = p->next) {
+	packfile_store_for_each_pack(packfiles, p) {
 		if (!p->pack_local || p->is_cruft)
 			continue;
 		if (limit) {
@@ -511,12 +511,12 @@ static int too_many_packs(struct gc_config *cfg)
 {
 	struct packfile_store *packs = the_repository->objects->packfiles;
 	struct packed_git *p;
-	int cnt;
+	int cnt = 0;
 
 	if (cfg->gc_auto_pack_limit <= 0)
 		return 0;
 
-	for (cnt = 0, p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		if (!p->pack_local)
 			continue;
 		if (p->pack_keep)
@@ -1425,9 +1425,9 @@ static int incremental_repack_auto_condition(struct gc_config *cfg UNUSED)
 	if (incremental_repack_auto_limit < 0)
 		return 1;
 
-	for (p = packfile_store_get_all_packs(the_repository->objects->packfiles);
-	     count < incremental_repack_auto_limit && p;
-	     p = p->next) {
+	packfile_store_for_each_pack(the_repository->objects->packfiles, p) {
+		if (count >= incremental_repack_auto_limit)
+			break;
 		if (!p->multi_pack_index)
 			count++;
 	}
@@ -1494,7 +1494,7 @@ static off_t get_auto_pack_size(void)
 	struct repository *r = the_repository;
 
 	odb_reprepare(r->objects);
-	for (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {
+	packfile_store_for_each_pack(r->objects->packfiles, p) {
 		if (p->pack_size > max_size) {
 			second_largest_size = max_size;
 			max_size = p->pack_size;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
@@ -3856,7 +3856,7 @@ static void read_packs_list_from_stdin(struct rev_info *revs)
 	string_list_sort(&exclude_packs);
 	string_list_remove_duplicates(&exclude_packs, 0);
 
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		const char *pack_name = pack_basename(p);
 
 		if ((item = string_list_lookup(&include_packs, pack_name)))
@@ -4107,7 +4107,7 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs
 	 * Re-mark only the fresh packs as kept so that objects in
 	 * unknown packs do not halt the reachability traversal early.
 	 */
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next)
+	packfile_store_for_each_pack(packs, p)
 		p->pack_keep_in_core = 0;
 	mark_pack_kept_in_core(fresh_packs, 1);
 
@@ -4145,7 +4145,7 @@ static void read_cruft_objects(void)
 	string_list_sort(&discard_packs);
 	string_list_sort(&fresh_packs);
 
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		const char *pack_name = pack_basename(p);
 		struct string_list_item *item;
 
@@ -4446,7 +4446,7 @@ static void loosen_unused_packed_objects(void)
 	uint32_t loosened_objects_nr = 0;
 	struct object_id oid;
 
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		if (!p->pack_local || p->pack_keep || p->pack_keep_in_core)
 			continue;
 
@@ -4753,7 +4753,7 @@ static void add_extra_kept_packs(const struct string_list *names)
 	if (!names->nr)
 		return;
 
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		const char *name = basename(p->pack_name);
 		int i;
 
@@ -5194,7 +5194,7 @@ int cmd_pack_objects(int argc,
 		struct packfile_store *packs = the_repository->objects->packfiles;
 		struct packed_git *p;
 
-		for (p = packfile_store_get_all_packs(packs); p; p = p->next)
+		packfile_store_for_each_pack(packs, p)
 			if (p->pack_local && p->pack_keep)
 				break;
 		if (!p) /* no keep-able packs found */
@@ -5209,7 +5209,7 @@ int cmd_pack_objects(int argc,
 		struct packfile_store *packs = the_repository->objects->packfiles;
 		struct packed_git *p;
 
-		for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+		packfile_store_for_each_pack(packs, p) {
 			if (!p->pack_local) {
 				have_non_local_packs = 1;
 				break;
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
@@ -567,28 +567,24 @@ static struct pack_list * add_pack(struct packed_git *p)
 static struct pack_list * add_pack_file(const char *filename)
 {
 	struct packfile_store *packs = the_repository->objects->packfiles;
-	struct packed_git *p = packfile_store_get_all_packs(packs);
+	struct packed_git *p;
 
 	if (strlen(filename) < 40)
 		die("Bad pack filename: %s", filename);
 
-	while (p) {
+	packfile_store_for_each_pack(packs, p)
 		if (strstr(p->pack_name, filename))
 			return add_pack(p);
-		p = p->next;
-	}
 	die("Filename %s not found in packed_git", filename);
 }
 
 static void load_all(void)
 {
 	struct packfile_store *packs = the_repository->objects->packfiles;
-	struct packed_git *p = packfile_store_get_all_packs(packs);
+	struct packed_git *p;
 
-	while (p) {
+	packfile_store_for_each_pack(packs, p)
 		add_pack(p);
-		p = p->next;
-	}
 }
 
 int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, struct repository *repo UNUSED) {
diff --git a/connected.c b/connected.c
@@ -77,7 +77,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
 			struct packfile_store *packs = the_repository->objects->packfiles;
 			struct packed_git *p;
 
-			for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+			packfile_store_for_each_pack(packs, p) {
 				if (!p->pack_promisor)
 					continue;
 				if (find_pack_entry_one(oid, p))
diff --git a/http-backend.c b/http-backend.c
@@ -609,13 +609,13 @@ static void get_info_packs(struct strbuf *hdr, char *arg UNUSED)
 	size_t cnt = 0;
 
 	select_getanyfile(hdr);
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		if (p->pack_local)
 			cnt++;
 	}
 
 	strbuf_grow(&buf, cnt * 53 + 2);
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		if (p->pack_local)
 			strbuf_addf(&buf, "P %s\n", p->pack_name + objdirlen + 6);
 	}
diff --git a/http.c b/http.c
@@ -2425,7 +2425,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head,
 	 * If we already have the pack locally, no need to fetch its index or
 	 * even add it to list; we already have all of its objects.
 	 */
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		if (hasheq(p->hash, sha1, the_repository->hash_algo))
 			return 0;
 	}
diff --git a/object-name.c b/object-name.c
@@ -213,9 +213,11 @@ static void find_short_packed_object(struct disambiguate_state *ds)
 			unique_in_midx(m, ds);
 	}
 
-	for (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;
-	     p = p->next)
+	packfile_store_for_each_pack(ds->repo->objects->packfiles, p) {
+		if (ds->ambiguous)
+			break;
 		unique_in_pack(p, ds);
+	}
 }
 
 static int finish_object_disambiguation(struct disambiguate_state *ds,
@@ -805,7 +807,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)
 			find_abbrev_len_for_midx(m, mad);
 	}
 
-	for (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)
+	packfile_store_for_each_pack(mad->repo->objects->packfiles, p)
 		find_abbrev_len_for_pack(p, mad);
 }
 
diff --git a/pack-bitmap.c b/pack-bitmap.c
@@ -664,7 +664,7 @@ static int open_pack_bitmap(struct repository *r,
 	struct packed_git *p;
 	int ret = -1;
 
-	for (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {
+	packfile_store_for_each_pack(r->objects->packfiles, p) {
 		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
 			ret = 0;
 			/*
@@ -3347,6 +3347,7 @@ static int verify_bitmap_file(const struct git_hash_algo *algop,
 int verify_bitmap_files(struct repository *r)
 {
 	struct odb_source *source;
+	struct packed_git *p;
 	int res = 0;
 
 	odb_prepare_alternates(r->objects);
@@ -3362,8 +3363,7 @@ int verify_bitmap_files(struct repository *r)
 		free(midx_bitmap_name);
 	}
 
-	for (struct packed_git *p = packfile_store_get_all_packs(r->objects->packfiles);
-	     p; p = p->next) {
+	packfile_store_for_each_pack(r->objects->packfiles, p) {
 		char *pack_bitmap_name = pack_bitmap_filename(p);
 		res |= verify_bitmap_file(r->hash_algo, pack_bitmap_name);
 		free(pack_bitmap_name);
diff --git a/pack-objects.c b/pack-objects.c
@@ -96,13 +96,13 @@ static void prepare_in_pack_by_idx(struct packing_data *pdata)
 	 * (i.e. in_pack_idx also zero) should return NULL.
 	 */
 	mapping[cnt++] = NULL;
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next, cnt++) {
+	packfile_store_for_each_pack(packs, p) {
 		if (cnt == nr) {
 			free(mapping);
 			return;
 		}
 		p->index = cnt;
-		mapping[cnt] = p;
+		mapping[cnt++] = p;
 	}
 	pdata->in_pack_by_idx = mapping;
 }
diff --git a/packfile.c b/packfile.c
@@ -2099,7 +2099,7 @@ struct packed_git **kept_pack_cache(struct repository *r, unsigned flags)
 		 * covers, one kept and one not kept, but the midx returns only
 		 * the non-kept version.
 		 */
-		for (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {
+		packfile_store_for_each_pack(r->objects->packfiles, p) {
 			if ((p->pack_keep && (flags & ON_DISK_KEEP_PACKS)) ||
 			    (p->pack_keep_in_core && (flags & IN_CORE_KEEP_PACKS))) {
 				ALLOC_GROW(packs, nr + 1, alloc);
@@ -2196,7 +2196,7 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb,
 	int r = 0;
 	int pack_errors = 0;
 
-	for (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next) {
+	packfile_store_for_each_pack(repo->objects->packfiles, p) {
 		if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)
 			continue;
 		if ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) &&
diff --git a/packfile.h b/packfile.h
@@ -136,6 +136,12 @@ void packfile_store_reprepare(struct packfile_store *store);
 void packfile_store_add_pack(struct packfile_store *store,
 			     struct packed_git *pack);
 
+/*
+ * Load and iterate through all packs of the given packfile store.
+ */
+#define packfile_store_for_each_pack(store, p) \
+	for (p = packfile_store_get_all_packs(store); p; p = p->next)
+
 /*
  * Get all packs managed by the given store, including packfiles that are
  * referenced by multi-pack indices.
diff --git a/repack-cruft.c b/repack-cruft.c
@@ -12,7 +12,7 @@ static void combine_small_cruft_packs(FILE *in, off_t combine_cruft_below_size,
 	struct strbuf buf = STRBUF_INIT;
 	size_t i;
 
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		if (!(p->is_cruft && p->pack_local))
 			continue;
 
diff --git a/repack-geometry.c b/repack-geometry.c
@@ -33,7 +33,7 @@ void pack_geometry_init(struct pack_geometry *geometry,
 	struct packed_git *p;
 	struct strbuf buf = STRBUF_INIT;
 
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		if (args->local && !p->pack_local)
 			/*
 			 * When asked to only repack local packfiles we skip
diff --git a/repack.c b/repack.c
@@ -128,7 +128,7 @@ void existing_packs_collect(struct existing_packs *existing,
 	struct packed_git *p;
 	struct strbuf buf = STRBUF_INIT;
 
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		size_t i;
 		const char *base;
 
diff --git a/server-info.c b/server-info.c
@@ -293,7 +293,7 @@ static void init_pack_info(struct repository *r, const char *infofile, int force
 	int i;
 	size_t alloc = 0;
 
-	for (p = packfile_store_get_all_packs(packs); p; p = p->next) {
+	packfile_store_for_each_pack(packs, p) {
 		/* we ignore things on alternate path since they are
 		 * not available to the pullers in general.
 		 */
diff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c
@@ -39,11 +39,12 @@ int cmd__find_pack(int argc, const char **argv)
 	if (repo_get_oid(the_repository, argv[0], &oid))
 		die("cannot parse %s as an object name", argv[0]);
 
-	for (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next)
+	packfile_store_for_each_pack(the_repository->objects->packfiles, p) {
 		if (find_pack_entry_one(&oid, p)) {
 			printf("%s\n", p->pack_name);
 			actual_count++;
 		}
+	}
 
 	if (count > -1 && count != actual_count)
 		die("bad packfile count %d instead of %d", actual_count, count);
diff --git a/t/helper/test-pack-mtimes.c b/t/helper/test-pack-mtimes.c
@@ -37,7 +37,7 @@ int cmd__pack_mtimes(int argc, const char **argv)
 	if (argc != 2)
 		usage(pack_mtimes_usage);
 
-	for (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next) {
+	packfile_store_for_each_pack(the_repository->objects->packfiles, p) {
 		strbuf_addstr(&buf, basename(p->pack_name));
 		strbuf_strip_suffix(&buf, ".pack");
 		strbuf_addstr(&buf, ".mtimes");

Original file line number	Diff line number	Diff line change
`@@ -609,13 +609,13 @@ static void get_info_packs(struct strbuf hdr, char arg UNUSED)`
`609`	`609`	`size_t cnt = 0;`
`610`	`610`
`611`	`611`	`select_getanyfile(hdr);`
`612`		`- for (p = packfile_store_get_all_packs(packs); p; p = p->next) {`
	`612`	`+ packfile_store_for_each_pack(packs, p) {`
`613`	`613`	`if (p->pack_local)`
`614`	`614`	`cnt++;`
`615`	`615`	`}`
`616`	`616`
`617`	`617`	`strbuf_grow(&buf, cnt * 53 + 2);`
`618`		`- for (p = packfile_store_get_all_packs(packs); p; p = p->next) {`
	`618`	`+ packfile_store_for_each_pack(packs, p) {`
`619`	`619`	`if (p->pack_local)`
`620`	`620`	`strbuf_addf(&buf, "P %s\n", p->pack_name + objdirlen + 6);`
`621`	`621`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2425,7 +2425,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head,`
`2425`	`2425`	`* If we already have the pack locally, no need to fetch its index or`
`2426`	`2426`	`* even add it to list; we already have all of its objects.`
`2427`	`2427`	`*/`
`2428`		`- for (p = packfile_store_get_all_packs(packs); p; p = p->next) {`
	`2428`	`+ packfile_store_for_each_pack(packs, p) {`
`2429`	`2429`	`if (hasheq(p->hash, sha1, the_repository->hash_algo))`
`2430`	`2430`	`return 0;`
`2431`	`2431`	`}`
Original file line number	Diff line number	Diff line change
`@@ -213,9 +213,11 @@ static void find_short_packed_object(struct disambiguate_state *ds)`
`213`	`213`	`unique_in_midx(m, ds);`
`214`	`214`	`}`
`215`	`215`
`216`		`- for (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;`
`217`		`- p = p->next)`
	`216`	`+ packfile_store_for_each_pack(ds->repo->objects->packfiles, p) {`
	`217`	`+ if (ds->ambiguous)`
	`218`	`+ break;`
`218`	`219`	`unique_in_pack(p, ds);`
	`220`	`+ }`
`219`	`221`	`}`
`220`	`222`
`221`	`223`	`static int finish_object_disambiguation(struct disambiguate_state *ds,`
`@@ -805,7 +807,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)`
`805`	`807`	`find_abbrev_len_for_midx(m, mad);`
`806`	`808`	`}`
`807`	`809`
`808`		`- for (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)`
	`810`	`+ packfile_store_for_each_pack(mad->repo->objects->packfiles, p)`
`809`	`811`	`find_abbrev_len_for_pack(p, mad);`
`810`	`812`	`}`
`811`	`813`