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

[PATCH/WIP 7/7] fast-import: fix data corruption in load_tree

From
Dmitry Ivankov <divanorama@gmail.com>
Date
Jul 28, 2011, 04:46 UTC
Message-ID
<1311828370-30477-8-git-send-email-divanorama@gmail.com>
In-Reply-To
<1311828370-30477-1-git-send-email-divanorama@gmail.com>

load_tree could be used to load a tree having different base and current sha1. For example it can happens after a parent tree was set by sha1 (it's tree becomes NULL, versions[0].sha1 remain and versions[1].sha1 change). But it doesn't look at versions[0].sha1 and just loads a new version resetting the base one to the new one. This corrupts parent tree delta.

Try to detect that case. Load both base and new trees and merge them together so that mktree is able to produce both base and new trees correctly.

There still may be a delta data corruption. For example tree_content_set with subtree != NULL can produce subtree entries bases and subtree's new parent base mismatch. tree_content_set is used in file_modify_cr - copy and move trees by names. And another place is notes writing thing that does some trees magic too.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
 fast-import.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 14a2a63..feccd14 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1389,14 +1389,6 @@ static void load_tree_content(struct tree_content **root, unsigned char *sha1)
 	free(buf);
 }
 
-static void load_tree(struct tree_entry *root)
-{
-	root->tree = t = new_tree_content(8);
-	if (is_null_sha1(sha1))
-		return;
-       load_tree_content(&root->tree, root->versions[1].sha1);
-}
-
 static int tecmp0 (const void *_a, const void *_b)
 {
 	struct tree_entry *a = *((struct tree_entry**)_a);
@@ -1442,6 +1434,66 @@ static void mktree(struct tree_content *t, int v, struct strbuf *b)
 	}
 }
 
+static void load_tree(struct tree_entry *root)
+{
+	struct tree_content *oldt;
+	size_t n, i, j;
+
+	root->tree = new_tree_content(8);
+	if (is_null_sha1(root->versions[1].sha1)) {
+		if (!S_ISDIR(root->versions[0].mode) || is_null_sha1(root->versions[0].sha1) || !hashcmp(root->versions[0].sha1, root->versions[1].sha1))
+			return;
+		// looks like it is currently unreachable, but let it be for a while
+		load_tree_content(&root->tree, root->versions[0].sha1);
+		for (i = 0; i < root->tree->entry_count; ++i) {
+			root->tree->entries[i]->versions[1].mode = 0;
+			hashclr(root->tree->entries[i]->versions[1].sha1);
+		}
+		return;
+	}
+
+	load_tree_content(&root->tree, root->versions[1].sha1);
+	if (!S_ISDIR(root->versions[0].mode) || is_null_sha1(root->versions[0].sha1) || !hashcmp(root->versions[0].sha1, root->versions[1].sha1))
+ 		return;
+
+	oldt = new_tree_content(8);
+	load_tree_content(&oldt, root->versions[0].sha1);
+
+	qsort(root->tree->entries, root->tree->entry_count, sizeof(root->tree->entries[0]), tecmp1);
+	qsort(oldt->entries, oldt->entry_count, sizeof(oldt->entries[0]), tecmp1);
+
+	n = root->tree->entry_count;
+	i = 0;
+	j = 0;
+	while (i < n || j < oldt->entry_count) {
+		int cmp = i == n ? 1 : j == oldt->entry_count ? -1 : tecmp1(root->tree->entries + i, oldt->entries + j);
+		if (cmp > 0) {
+			if (root->tree->entry_count == root->tree->entry_capacity)
+				root->tree = grow_tree_content(root->tree, root->tree->entry_count);
+			oldt->entries[j]->versions[1].mode = 0;
+			hashclr(oldt->entries[j]->versions[1].sha1);
+			root->tree->entries[root->tree->entry_count++] = oldt->entries[j];
+			oldt->entries[j] = NULL;
+			++j;
+		} else if (cmp < 0) {
+			root->tree->entries[i]->versions[0].mode = 0;
+			hashclr(root->tree->entries[i]->versions[0].sha1);
+			++i;
+		} else {
+			root->tree->entries[i]->versions[0].mode = oldt->entries[j]->versions[1].mode;
+			hashcpy(root->tree->entries[i]->versions[0].sha1, oldt->entries[j]->versions[1].sha1);
+			++i;
+			++j;
+		}
+	}
+	for (j = 0; j < oldt->entry_count; ++j)
+		if (oldt->entries[j]) {
+			release_tree_entry(oldt->entries[j]);
+			oldt->entries[j] = NULL;
+		}
+	release_tree_content(oldt);
+}
+
 static void drop_old(struct tree_entry *root)
 {
 	struct tree_content *t = root->tree;
-- 
1.7.3.4
Previous: Jonathan Nieder
Message 13 of 13 in “was: long fast-import errors out "failed to apply delta"”
  1. 0/7 was: long fast-import errors out "failed to apply delta"Dmitry Ivankov, Jul 28, 2011
  2. 1/7 fast-import: extract object preparation functionDmitry Ivankov, Jul 28, 2011
  3. 2/7 fast-import: be saner with temporary treesDmitry Ivankov, Jul 28, 2011
  4. Jonathan NiederJul 28, 2011
  5. 3/7 fast-import: fix a data corruption in parse_lsDmitry Ivankov, Jul 28, 2011
  6. Jonathan NiederJul 28, 2011
  7. 4/7 fast-import: fix data corruption in store_treeDmitry Ivankov, Jul 28, 2011
  8. Jonathan NiederJul 28, 2011
  9. Dmitry IvankovJul 28, 2011
  10. 5/7 fast-import: extract tree_content reading functionDmitry Ivankov, Jul 28, 2011
  11. 6/7 fast-import: workaround data corruptionDmitry Ivankov, Jul 28, 2011
  12. Jonathan NiederJul 28, 2011
  13. 7/7 fast-import: fix data corruption in load_treeDmitry Ivankov, Jul 28, 2011

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.