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>

1 files changed, 112 insertions, 76 deletions

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,