sha1_file.c: the global packed_git variable static to the filesz/packed-git-static

This is a first step in making the codebase thread-safe.  By and
large, the operations which might benefit from threading are those
that work with pack files (e.g., checkout, blame), so the focus of
this patch is stop leaking the global list of pack files outside of
sha1_file.c.

The next step will be to control access to the list of pack files
with a mutex.  However, that alone is not enough to make pack file
access thread safe.  Even in a read-only operation, the window list
associated with each pack file will need to be controlled.
Additionally, the global counters in sha1_file.c will need to be
controlled.

This patch is a pure refactor with no functional changes, so it
shouldn't require any additional tests.  Adding the actual locks
will be a functional change, and will require additional tests.

[jc: with minimul style fixes before a full review]

Signed-off-by: Stefan Zager <szager@chromium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

5 files changed, 203 insertions, 138 deletions

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a7f70cb858..c1a6a2c254 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -83,14 +83,32 @@ static char const * const count_objects_usage[] = {
 	NULL
 };
 
+struct pack_data {
+	unsigned long packed;
+	off_t size_pack;
+	unsigned long num_pack;
+};
+
+static int collect_pack_data(struct packed_git *p, void *data)
+{
+	struct pack_data *pd = (struct pack_data *) data;
+	if (p->pack_local && !open_pack_index(p)) {
+		pd->packed += p->num_objects;
+		pd->size_pack += p->pack_size + p->index_size;
+		pd->num_pack++;
+	}
+	return 0;
+}
+
 int cmd_count_objects(int argc, const char **argv, const char *prefix)
 {
 	int i, verbose = 0, human_readable = 0;
 	const char *objdir = get_object_directory();
 	int len = strlen(objdir);
 	char *path = xmalloc(len + 50);
-	unsigned long loose = 0, packed = 0, packed_loose = 0;
+	unsigned long loose = 0, packed_loose = 0;
 	off_t loose_size = 0;
+	struct pack_data pd = {0, 0, 0};
 	struct option opts[] = {
 		OPT__VERBOSE(&verbose, N_("be verbose")),
 		OPT_BOOL('H', "human-readable", &human_readable,
@@ -118,41 +136,29 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 		closedir(d);
 	}
 	if (verbose) {
-		struct packed_git *p;
-		unsigned long num_pack = 0;
-		off_t size_pack = 0;
 		struct strbuf loose_buf = STRBUF_INIT;
 		struct strbuf pack_buf = STRBUF_INIT;
 		struct strbuf garbage_buf = STRBUF_INIT;
-		if (!packed_git)
-			prepare_packed_git();
-		for (p = packed_git; p; p = p->next) {
-			if (!p->pack_local)
-				continue;
-			if (open_pack_index(p))
-				continue;
-			packed += p->num_objects;
-			size_pack += p->pack_size + p->index_size;
-			num_pack++;
-		}
+		prepare_packed_git();
+		foreach_packed_git(collect_pack_data, NULL, &pd);
 
 		if (human_readable) {
 			strbuf_humanise_bytes(&loose_buf, loose_size);
-			strbuf_humanise_bytes(&pack_buf, size_pack);
+			strbuf_humanise_bytes(&pack_buf, pd.size_pack);
 			strbuf_humanise_bytes(&garbage_buf, size_garbage);
 		} else {
 			strbuf_addf(&loose_buf, "%lu",
 				    (unsigned long)(loose_size / 1024));
 			strbuf_addf(&pack_buf, "%lu",
-				    (unsigned long)(size_pack / 1024));
+				    (unsigned long)(pd.size_pack / 1024));
 			strbuf_addf(&garbage_buf, "%lu",
 				    (unsigned long)(size_garbage / 1024));
 		}
 
 		printf("count: %lu\n", loose);
 		printf("size: %s\n", loose_buf.buf);
-		printf("in-pack: %lu\n", packed);
-		printf("packs: %lu\n", num_pack);
+		printf("in-pack: %lu\n", pd.packed);
+		printf("packs: %lu\n", pd.num_pack);
 		printf("size-pack: %s\n", pack_buf.buf);
 		printf("prune-packable: %lu\n", packed_loose);
 		printf("garbage: %lu\n", garbage);
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1affdd5e92..e55f0803a2 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -624,6 +624,29 @@ static struct option fsck_opts[] = {
 	OPT_END(),
 };
 
+struct verify_packs_data {
+	struct progress *progress;
+	uint32_t total;
+	uint32_t count;
+};
+
+static int count_packs(struct packed_git *p, void *data)
+{
+	struct verify_packs_data *vpd = (struct verify_packs_data *) data;
+	if (!open_pack_index(p))
+		vpd->total++;
+	return 0;
+}
+
+static int verify_packs(struct packed_git *p, void *data)
+{
+	struct verify_packs_data *vpd = (struct verify_packs_data *) data;
+	if (verify_pack(p, fsck_obj_buffer, vpd->progress, vpd->count))
+		errors_found |= ERROR_PACK;
+	vpd->count += p->num_objects;
+	return 0;
+}
+
 int cmd_fsck(int argc, const char **argv, const char *prefix)
 {
 	int i, heads;
@@ -657,29 +680,16 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 	}
 
 	if (check_full) {
-		struct packed_git *p;
-		uint32_t total = 0, count = 0;
-		struct progress *progress = NULL;
+		struct verify_packs_data vpd = {0, 0, 0};
 
 		prepare_packed_git();
 
 		if (show_progress) {
-			for (p = packed_git; p; p = p->next) {
-				if (open_pack_index(p))
-					continue;
-				total += p->num_objects;
-			}
-
-			progress = start_progress("Checking objects", total);
-		}
-		for (p = packed_git; p; p = p->next) {
-			/* verify gives error messages itself */
-			if (verify_pack(p, fsck_obj_buffer,
-					progress, count))
-				errors_found |= ERROR_PACK;
-			count += p->num_objects;
+			foreach_packed_git(count_packs, NULL, &vpd);
+			vpd.progress = start_progress("Checking objects", vpd.total);
 		}
-		stop_progress(&progress);
+		foreach_packed_git(verify_packs, NULL, &vpd);
+		stop_progress(&vpd.progress);
 	}
 
 	heads = 0;
diff --git a/builtin/gc.c b/builtin/gc.c
index c19545d49e..d9fbbd711e 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -125,26 +125,26 @@ static int too_many_loose_objects(void)
 	return needed;
 }
 
+static int count_packs(struct packed_git *p, void *data)
+{
+	/*
+	 * Perhaps check the size of the pack and count only
+	 * very small ones here?
+	 */
+	if (p->pack_local && !p->pack_keep)
+		(*((int *) data))++;
+	return 0;
+}
+
 static int too_many_packs(void)
 {
-	struct packed_git *p;
-	int cnt;
+	int cnt = 0;
 
 	if (gc_auto_pack_limit <= 0)
 		return 0;
 
 	prepare_packed_git();
-	for (cnt = 0, p = packed_git; p; p = p->next) {
-		if (!p->pack_local)
-			continue;
-		if (p->pack_keep)
-			continue;
-		/*
-		 * Perhaps check the size of the pack and count only
-		 * very small ones here?
-		 */
-		cnt++;
-	}
+	foreach_packed_git(count_packs, NULL, &cnt);
 	return gc_auto_pack_limit <= cnt;
 }
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 541667f102..bc3074b30c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -900,14 +900,45 @@ static int no_try_delta(const char *path)
 	return 0;
 }
 
+struct find_pack_data {
+	const unsigned char *sha1;
+	off_t offset;
+	struct packed_git *found_pack;
+	int exclude;
+	int found_non_local_pack;
+	int found_pack_keep;
+};
+
+static int find_pack_fn(struct packed_git *p, void *data)
+{
+	struct find_pack_data *fpd = (struct find_pack_data *) data;
+	off_t offset = find_pack_entry_one(fpd->sha1, p);
+	if (offset) {
+		if (!fpd->found_pack) {
+			if (!is_pack_valid(p)) {
+				warning("packfile %s cannot be accessed", p->pack_name);
+				return 0;
+			}
+			fpd->offset = offset;
+			fpd->found_pack = p;
+		}
+		if (fpd->exclude)
+			return 1;
+		if (!p->pack_local)
+			fpd->found_non_local_pack = 1;
+		else if (p->pack_keep)
+			fpd->found_pack_keep = 1;
+	}
+	return 0;
+}
+
 static int add_object_entry(const unsigned char *sha1, enum object_type type,
 			    const char *name, int exclude)
 {
 	struct object_entry *entry;
-	struct packed_git *p, *found_pack = NULL;
-	off_t found_offset = 0;
 	int ix;
 	uint32_t hash = name_hash(name);
+	struct find_pack_data fpd;
 
 	ix = nr_objects ? locate_object_entry_hash(sha1) : -1;
 	if (ix >= 0) {
@@ -923,27 +954,18 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
 	if (!exclude && local && has_loose_object_nonlocal(sha1))
 		return 0;
 
-	for (p = packed_git; p; p = p->next) {
-		off_t offset = find_pack_entry_one(sha1, p);
-		if (offset) {
-			if (!found_pack) {
-				if (!is_pack_valid(p)) {
-					warning("packfile %s cannot be accessed", p->pack_name);
-					continue;
-				}
-				found_offset = offset;
-				found_pack = p;
-			}
-			if (exclude)
-				break;
-			if (incremental)
-				return 0;
-			if (local && !p->pack_local)
-				return 0;
-			if (ignore_packed_keep && p->pack_local && p->pack_keep)
-				return 0;
-		}
-	}
+	fpd.sha1 = sha1;
+	fpd.offset = 0;
+	fpd.found_pack = NULL;
+	fpd.exclude = exclude;
+	fpd.found_non_local_pack = fpd.found_pack_keep = 0;
+
+	foreach_packed_git(find_pack_fn, NULL, &fpd);
+	if (fpd.found_pack && !exclude &&
+	    (incremental ||
+	     (local && fpd.found_non_local_pack) ||
+	     (ignore_packed_keep && fpd.found_pack_keep)))
+		return 0;
 
 	if (nr_objects >= nr_alloc) {
 		nr_alloc = (nr_alloc  + 1024) * 3 / 2;
@@ -960,9 +982,9 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
 		entry->preferred_base = 1;
 	else
 		nr_result++;
-	if (found_pack) {
-		entry->in_pack = found_pack;
-		entry->in_pack_offset = found_offset;
+	if (fpd.found_pack) {
+		entry->in_pack = fpd.found_pack;
+		entry->in_pack_offset = fpd.offset;
 	}
 
 	if (object_ix_hashsz * 3 <= nr_objects * 4)
@@ -2257,35 +2279,40 @@ static int ofscmp(const void *a_, const void *b_)
 		return hashcmp(a->object->sha1, b->object->sha1);
 }
 
-static void add_objects_in_unpacked_packs(struct rev_info *revs)
+static int add_objects_fn(struct packed_git *p, void *data)
 {
-	struct packed_git *p;
-	struct in_pack in_pack;
+	struct in_pack *ip = (struct in_pack *) data;
 	uint32_t i;
+	const unsigned char *sha1;
+	struct object *o;
 
-	memset(&in_pack, 0, sizeof(in_pack));
-
-	for (p = packed_git; p; p = p->next) {
-		const unsigned char *sha1;
-		struct object *o;
-
-		if (!p->pack_local || p->pack_keep)
-			continue;
+	if (p->pack_local && !p->pack_keep) {
 		if (open_pack_index(p))
 			die("cannot open pack index");
 
-		ALLOC_GROW(in_pack.array,
-			   in_pack.nr + p->num_objects,
-			   in_pack.alloc);
+		ALLOC_GROW(ip->array,
+			   ip->nr + p->num_objects,
+			   ip->alloc);
 
 		for (i = 0; i < p->num_objects; i++) {
 			sha1 = nth_packed_object_sha1(p, i);
 			o = lookup_unknown_object(sha1);
 			if (!(o->flags & OBJECT_ADDED))
-				mark_in_pack_object(o, p, &in_pack);
+				mark_in_pack_object(o, p, ip);
 			o->flags |= OBJECT_ADDED;
 		}
 	}
+	return 0;
+}
+
+static void add_objects_in_unpacked_packs(struct rev_info *revs)
+{
+	struct in_pack in_pack;
+	uint32_t i;
+
+	memset(&in_pack, 0, sizeof(in_pack));
+
+	foreach_packed_git(add_objects_fn, NULL, &in_pack);
 
 	if (in_pack.nr) {
 		qsort(in_pack.array, in_pack.nr, sizeof(in_pack.array[0]),
@@ -2298,54 +2325,63 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)
 	free(in_pack.array);
 }
 
+struct nonlocal_or_keep_data {
+	struct packed_git **last_found;
+	const unsigned char *sha1;
+	int found;
+};
+
+static int find_nonlocal_or_keep_fn(struct packed_git *p, void *data)
+{
+	struct nonlocal_or_keep_data *d = (struct nonlocal_or_keep_data *) data;
+	if ((!p->pack_local || p->pack_keep) &&
+	    find_pack_entry_one(d->sha1, p)) {
+		*d->last_found = p;
+		d->found = 1;
+		return 1;
+	}
+	return 0;
+}
+
 static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
 {
 	static struct packed_git *last_found = (void *)1;
-	struct packed_git *p;
+	struct nonlocal_or_keep_data d;
+	struct packed_git *hint;
 
-	p = (last_found != (void *)1) ? last_found : packed_git;
+	d.last_found = &last_found;
+	d.sha1 = sha1;
+	d.found = 0;
 
-	while (p) {
-		if ((!p->pack_local || p->pack_keep) &&
-			find_pack_entry_one(sha1, p)) {
-			last_found = p;
-			return 1;
-		}
-		if (p == last_found)
-			p = packed_git;
-		else
-			p = p->next;
-		if (p == last_found)
-			p = p->next;
-	}
-	return 0;
+	hint = (last_found != (void *)1) ? last_found : NULL;
+
+	foreach_packed_git(find_nonlocal_or_keep_fn, hint, &d);
+	return d.found;
 }
 
-static void loosen_unused_packed_objects(struct rev_info *revs)
+static int loosen_unused_objects_fn(struct packed_git *p, void *data)
 {
-	struct packed_git *p;
+	const unsigned char *sha1 = (const unsigned char *) data;
 	uint32_t i;
-	const unsigned char *sha1;
 
-	for (p = packed_git; p; p = p->next) {
-		if (!p->pack_local || p->pack_keep)
-			continue;
+	if (!p->pack_local || p->pack_keep)
+		return 0;
 
-		if (unpack_unreachable_expiration &&
-		    p->mtime < unpack_unreachable_expiration)
-			continue;
+	if (unpack_unreachable_expiration &&
+		p->mtime < unpack_unreachable_expiration)
+		return 0;
 
-		if (open_pack_index(p))
-			die("cannot open pack index");
+	if (open_pack_index(p))
+		die("cannot open pack index");
 
-		for (i = 0; i < p->num_objects; i++) {
-			sha1 = nth_packed_object_sha1(p, i);
-			if (!locate_object_entry(sha1) &&
-				!has_sha1_pack_kept_or_nonlocal(sha1))
-				if (force_object_loose(sha1, p->mtime))
-					die("unable to force loose object");
-		}
+	for (i = 0; i < p->num_objects; i++) {
+		sha1 = nth_packed_object_sha1(p, i);
+		if (!locate_object_entry(sha1) &&
+			!has_sha1_pack_kept_or_nonlocal(sha1))
+			if (force_object_loose(sha1, p->mtime))
+				die("unable to force loose object");
 	}
+	return 0;
 }
 
 static void get_object_list(int ac, const char **av)
@@ -2383,7 +2419,7 @@ static void get_object_list(int ac, const char **av)
 	if (keep_unreachable)
 		add_objects_in_unpacked_packs(&revs);
 	if (unpack_unreachable)
-		loosen_unused_packed_objects(&revs);
+		foreach_packed_git(loosen_unused_objects_fn, NULL, NULL);
 }
 
 static int option_parse_index_version(const struct option *opt,
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 649c3aaa93..0a86132af9 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -567,29 +567,42 @@ static struct pack_list * add_pack(struct packed_git *p)
 		return pack_list_insert(&altodb_packs, &l);
 }
 
+struct add_pack_data {
+	const char *filename;
+	int found;
+	struct pack_list *added_pack;
+};
+
+static int add_pack_fn(struct packed_git *p, void *data)
+{
+	struct add_pack_data *apd = (struct add_pack_data *) data;
+	if (apd->filename && strstr(p->pack_name, apd->filename)) {
+		apd->found = 1;
+		apd->added_pack = add_pack(p);
+		return 1;
+	} else if (!apd->filename) {
+		add_pack(p);
+	}
+	return 0;
+}
+
 static struct pack_list * add_pack_file(const char *filename)
 {
-	struct packed_git *p = packed_git;
+	struct add_pack_data apd = {filename, 0, NULL};
 
 	if (strlen(filename) < 40)
 		die("Bad pack filename: %s", filename);
 
-	while (p) {
-		if (strstr(p->pack_name, filename))
-			return add_pack(p);
-		p = p->next;
-	}
+	foreach_packed_git(add_pack_fn, NULL, &apd);
+	if (apd.found)
+		return apd.added_pack;
 	die("Filename %s not found in packed_git", filename);
 }
 
 static void load_all(void)
 {
-	struct packed_git *p = packed_git;
-
-	while (p) {
-		add_pack(p);
-		p = p->next;
-	}
+	struct add_pack_data apd = {NULL, 0, NULL};
+	foreach_packed_git(add_pack_fn, NULL, &apd);
 }
 
 int cmd_pack_redundant(int argc, const char **argv, const char *prefix)