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

[PATCH 3/3] Use helper function for copying index entry information

From
Linus Torvalds <torvalds@linux-foundation.org>
Date
Feb 23, 2008, 04:41 UTC
Message-ID
<alpine.LFD.1.00.0802222039250.21332@woody.linux-foundation.org>
In-Reply-To
<alpine.LFD.1.00.0802222037560.21332@woody.linux-foundation.org>
From: Linus Torvalds <torvalds@linux-foundation.org>

We used to just memcpy() the index entry when we copied the stat() and SHA1 hash information, which worked well enough back when the index entry was just an exact bit-for-bit representation of the information on disk.

However, these days we actually have various management information in the cache entry too, and we should be careful to not overwrite it when we copy the stat information from another index entry.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
It's still not the prettiest function ever, but I think it's obvious 
enough what it does. And the call-site is prettier.
 cache.h        |   18 +++++++++++++++++-
 unpack-trees.c |    2 +-
 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index 88d3b92..fa5a9e5 100644
--- a/cache.h
+++ b/cache.h
@@ -110,7 +110,6 @@ struct ondisk_cache_entry {
 };
 
 struct cache_entry {
-	struct cache_entry *next;
 	unsigned int ce_ctime;
 	unsigned int ce_mtime;
 	unsigned int ce_dev;
@@ -121,6 +120,7 @@ struct cache_entry {
 	unsigned int ce_size;
 	unsigned int ce_flags;
 	unsigned char sha1[20];
+	struct cache_entry *next;
 	char name[FLEX_ARRAY]; /* more */
 };
 
@@ -138,6 +138,22 @@ struct cache_entry {
 #define CE_UNHASHED  (0x200000)
 
 /*
+ * Copy the sha1 and stat state of a cache entry from one to
+ * another. But we never change the name, or the hash state!
+ */
+#define CE_STATE_MASK (CE_HASHED | CE_UNHASHED)
+static inline void copy_cache_entry(struct cache_entry *dst, struct cache_entry *src)
+{
+	unsigned int state = dst->ce_flags & CE_STATE_MASK;
+
+	/* Don't copy hash chain and name */
+	memcpy(dst, src, offsetof(struct cache_entry, next));
+
+	/* Restore the hash state */
+	dst->ce_flags = (dst->ce_flags & ~CE_STATE_MASK) | state;
+}
+
+/*
  * We don't actually *remove* it, we can just mark it invalid so that
  * we won't find it in lookups.
  *
diff --git a/unpack-trees.c b/unpack-trees.c
index ec558f9..07c4c28 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -590,7 +590,7 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 		 * a match.
 		 */
 		if (same(old, merge)) {
-			memcpy(merge, old, offsetof(struct cache_entry, name));
+			copy_cache_entry(merge, old);
 		} else {
 			verify_uptodate(old, o);
 			invalidate_ce_path(old);
Previous: Linus Torvalds
Message 4 of 4 in “Fix up some name hashing uglies..”
  1. 0/3 Fix up some name hashing uglies..Linus Torvalds, Feb 23, 2008
  2. Fix name re-hashing semanticsLinus Torvalds, Feb 23, 2008
  3. 2/3 Name hash fixups: export (and rename) remove_hash_entryLinus Torvalds, Feb 23, 2008
  4. 3/3 Use helper function for copying index entry informationLinus Torvalds, Feb 23, 2008

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.