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

[PATCH] unpack_entry: invalidate newly added cache entry in case of error

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Apr 30, 2013, 02:29 UTC
Message-ID
<1367288992-14979-1-git-send-email-pclouds@gmail.com>

In this particular code path, we add "base" to the delta base cache. Then decide to free it, but we forgot about a dangling pointer in the cache. Invalidate that entry when we free "base".

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Some of my changes triggered a double free fault at "free(base);" in
 t5303. This looks like a correct thing to do, but I may be missing
 something (I'm not even sure how it happened). Please check.
 sha1_file.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 64228a2..99ead7c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1912,7 +1912,8 @@ void clear_delta_base_cache(void)
 		release_delta_base_cache(&delta_base_cache[p]);
 }
 
-static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
+static struct delta_base_cache_entry *
+add_delta_base_cache(struct packed_git *p, off_t base_offset,
 	void *base, unsigned long base_size, enum object_type type)
 {
 	unsigned long hash = pack_entry_hash(p, base_offset);
@@ -1947,6 +1948,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
 	ent->lru.prev = delta_base_cache_lru.prev;
 	delta_base_cache_lru.prev->next = &ent->lru;
 	delta_base_cache_lru.prev = &ent->lru;
+	return ent;
 }
 
 static void *read_object(const unsigned char *sha1, enum object_type *type,
@@ -2086,12 +2088,13 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 		void *delta_data;
 		void *base = data;
 		unsigned long delta_size, base_size = size;
+		struct delta_base_cache_entry *ent = NULL;
 		int i;
 
 		data = NULL;
 
 		if (base)
-			add_delta_base_cache(p, obj_offset, base, base_size, type);
+			ent = add_delta_base_cache(p, obj_offset, base, base_size, type);
 
 		if (!base) {
 			/*
@@ -2129,6 +2132,8 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 			      "at offset %"PRIuMAX" from %s",
 			      (uintmax_t)curpos, p->pack_name);
 			free(base);
+			if (ent)
+				ent->data = NULL;
 			data = NULL;
 			continue;
 		}
-- 
1.8.2.83.gc99314b
Next: Thomas Rast
Message 1 of 6 in “unpack_entry: invalidate newly added cache entry in case of error”
  1. unpack_entry: invalidate newly added cache entry in case of errorNguyễn Thái Ngọc Duy, Apr 30, 2013
  2. Thomas RastApr 30, 2013
  3. Duy NguyenApr 30, 2013
  4. Thomas RastApr 30, 2013
  5. Duy NguyenApr 30, 2013
  6. Junio C HamanoApr 30, 2013

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.