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

[PATCH v3 2/2] find_pack_entry(): do not keep packed_git pointer locally

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Feb 1, 2012, 13:48 UTC
Message-ID
<1328104135-475-2-git-send-email-pclouds@gmail.com>
In-Reply-To
<1328104135-475-1-git-send-email-pclouds@gmail.com>

Commit f7c22cc (always start looking up objects in the last used pack first - 2007-05-30) introduce a static packed_git* pointer as an optimization. The kept pointer however may become invalid if free_pack_by_name() happens to free that particular pack.

Current code base does not access packs after calling free_pack_by_name() so it should not be a problem. Anyway, move the pointer out so that free_pack_by_name() can reset it to avoid running into troubles in future.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Since Junio's already done the hard work. It'd be silly of me not to
 take advantage and credit for free :)
 The new loop looks much better.
 sha1_file.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index ff5bf42..ebe77b3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -54,6 +54,8 @@ static struct cached_object empty_tree = {
 	0
 };
 
+static struct packed_git *last_found_pack;
+
 static struct cached_object *find_cached_object(const unsigned char *sha1)
 {
 	int i;
@@ -720,6 +722,8 @@ void free_pack_by_name(const char *pack_name)
 			close_pack_index(p);
 			free(p->bad_object_sha1);
 			*pp = p->next;
+			if (last_found_pack == p)
+				last_found_pack = NULL;
 			free(p);
 			return;
 		}
@@ -2044,27 +2048,22 @@ static int find_pack_entry_1(const unsigned char *sha1,
 
 static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
 {
-	static struct packed_git *last_found = (void *)1;
 	struct packed_git *p;
 
 	prepare_packed_git();
 	if (!packed_git)
 		return 0;
-	p = (last_found == (void *)1) ? packed_git : last_found;
 
-	do {
-		if (find_pack_entry_1(sha1, p, e)) {
-			last_found = p;
-			return 1;
-		}
+	if (last_found_pack && find_pack_entry_1(sha1, last_found_pack, e))
+		return 1;
 
-		if (p == last_found)
-			p = packed_git;
-		else
-			p = p->next;
-		if (p == last_found)
-			p = p->next;
-	} while (p);
+	for (p = packed_git; p; p = p->next) {
+		if (p == last_found_pack || !find_pack_entry_1(sha1, p, e))
+			continue;
+
+		last_found_pack = p;
+		return 1;
+	}
 	return 0;
 }
 
-- 
1.7.8.36.g69ee2
Previous: Nguyễn Thái Ngọc DuyNext: Nicolas Pitre
Message 8 of 14 in “find_pack_entry(): do not keep packed_git pointer locally”
  1. find_pack_entry(): do not keep packed_git pointer locallyNguyễn Thái Ngọc Duy, Jan 30, 2012
  2. Junio C HamanoJan 30, 2012
  3. Nguyen Thai Ngoc DuyJan 31, 2012
  4. Nicolas PitreJan 31, 2012
  5. Junio C HamanoJan 31, 2012
  6. Nicolas PitreJan 31, 2012
  7. 1/2 Factor find_pack_entry()'s core outNguyễn Thái Ngọc Duy, Feb 1, 2012
  8. 2/2 find_pack_entry(): do not keep packed_git pointer locallyNguyễn Thái Ngọc Duy, Feb 1, 2012
  9. Nicolas PitreFeb 1, 2012
  10. 2/2 find_pack_entry(): do not keep packed_git pointer locallyNguyễn Thái Ngọc Duy, Feb 2, 2012
  11. Nicolas PitreFeb 1, 2012
  12. Junio C HamanoFeb 1, 2012
  13. Junio C HamanoFeb 1, 2012
  14. Nicolas PitreFeb 1, 2012

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.