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

[PATCH 2/3] unpack-trees: fix path search bug in verify_absent

From
Clemens Buchacher <drizzd@aon.at>
Date
Jan 1, 2009, 20:54 UTC
Message-ID
<1230843273-11056-3-git-send-email-drizzd@aon.at>
In-Reply-To
<1230843273-11056-2-git-send-email-drizzd@aon.at>

Commit 0cf73755 (unpack-trees.c: assume submodules are clean during check-out) changed an argument to verify_absent from 'path' to 'ce', which is however shadowed by a local variable of the same name.

The bug triggers if verify_absent is used on a tree entry, for which
the index contains one or more subsequent directories of the same
length. The affected subdirectories are removed from the index. The
testcase included in this commit bisects to 55218834 (checkout: do not
lose staged removal), which reveals the bug in this case, but is
otherwise unrelated.
---
 t/t1001-read-tree-m-2way.sh |   27 +++++++++++++++++++++++++++
 unpack-trees.c              |   23 ++++++++++++-----------
 2 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh
index 7f6ab31..271bc4e 100755
--- a/t/t1001-read-tree-m-2way.sh
+++ b/t/t1001-read-tree-m-2way.sh
@@ -365,4 +365,31 @@ test_expect_success \
      git ls-files --stage &&
      test -f a/b'
 
+test_expect_success \
+    'a/b vs a, plus c/d case setup.' \
+    'rm -f .git/index &&
+     rm -fr a &&
+     : >a &&
+     mkdir c &&
+     : >c/d &&
+     git update-index --add a c/d &&
+     treeM=`git write-tree` &&
+     echo treeM $treeM &&
+     git ls-tree $treeM &&
+     git ls-files --stage >treeM.out &&
+
+     rm -f a &&
+     mkdir a
+     : >a/b &&
+     git update-index --add --remove a a/b &&
+     treeH=`git write-tree` &&
+     echo treeH $treeH &&
+     git ls-tree $treeH'
+
+test_expect_success \
+    'a/b vs a, plus c/d case test.' \
+    'git read-tree -u -m "$treeH" "$treeM" &&
+     git ls-files --stage | tee >treeMcheck.out &&
+     test_cmp treeM.out treeMcheck.out'
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index a736947..f8e2484 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -289,7 +289,8 @@ static int unpack_nondirectories(int n, unsigned long mask, unsigned long dirmas
 	return 0;
 }
 
-static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *names, struct traverse_info *info)
+static int unpack_callback(int n, unsigned long mask, unsigned long dirmask,
+		struct name_entry *names, struct traverse_info *info)
 {
 	struct cache_entry *src[5] = { NULL, };
 	struct unpack_trees_options *o = info->data;
@@ -517,22 +518,22 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
 	namelen = strlen(ce->name);
 	pos = index_name_pos(o->src_index, ce->name, namelen);
 	if (0 <= pos)
-		return cnt; /* we have it as nondirectory */
+		return 0; /* we have it as nondirectory */
 	pos = -pos - 1;
 	for (i = pos; i < o->src_index->cache_nr; i++) {
-		struct cache_entry *ce = o->src_index->cache[i];
-		int len = ce_namelen(ce);
+		struct cache_entry *ce2 = o->src_index->cache[i];
+		int len = ce_namelen(ce2);
 		if (len < namelen ||
-		    strncmp(ce->name, ce->name, namelen) ||
-		    ce->name[namelen] != '/')
+		    strncmp(ce->name, ce2->name, namelen) ||
+		    ce2->name[namelen] != '/')
 			break;
 		/*
-		 * ce->name is an entry in the subdirectory.
+		 * ce2->name is an entry in the subdirectory.
 		 */
-		if (!ce_stage(ce)) {
-			if (verify_uptodate(ce, o))
+		if (!ce_stage(ce2)) {
+			if (verify_uptodate(ce2, o))
 				return -1;
-			add_entry(o, ce, CE_REMOVE, 0);
+			add_entry(o, ce2, CE_REMOVE, 0);
 		}
 		cnt++;
 	}
@@ -624,7 +625,7 @@ static int verify_absent(struct cache_entry *ce, const char *action,
 			 * If this removed entries from the index,
 			 * what that means is:
 			 *
-			 * (1) the caller unpack_trees_rec() saw path/foo
+			 * (1) the caller unpack_callback() saw path/foo
 			 * in the index, and it has not removed it because
 			 * it thinks it is handling 'path' as blob with
 			 * D/F conflict;
-- 
1.6.1
Previous: Clemens BuchacherNext: Clemens Buchacher
Message 3 of 20 in “unpack-trees: fix D/F conflict bugs in verify_absent”
  1. Clemens BuchacherJan 1, 2009
  2. 1/3 unpack-trees: handle failure in verify_absentClemens Buchacher, Jan 1, 2009
  3. 2/3 unpack-trees: fix path search bug in verify_absentClemens Buchacher, Jan 1, 2009
  4. 3/3 unpack-trees: remove redundant path search in verify_absentClemens Buchacher, Jan 1, 2009
  5. Junio C HamanoJan 6, 2009
  6. Johannes SchindelinJan 2, 2009
  7. Miklos VajnaJan 3, 2009
  8. Johannes SchindelinJan 2, 2009
  9. Clemens BuchacherJan 3, 2009
  10. Johannes SchindelinJan 3, 2009
  11. Junio C HamanoJan 4, 2009
  12. Clemens BuchacherJan 4, 2009
  13. Johannes SchindelinJan 6, 2009
  14. 0/2 unpack-trees: handle lstat failures in verify_absentJonathan Nieder, Jan 13, 2011
  15. 1/2 unpack-trees: handle lstat failure for existing directoryJonathan Nieder, Jan 13, 2011
  16. Miles BaderJan 13, 2011
  17. Jonathan NiederJan 13, 2011
  18. 2/2 unpack-trees: handle lstat failure for existing fileJonathan Nieder, Jan 13, 2011
  19. Clemens BuchacherJan 13, 2011
  20. Junio C HamanoJan 1, 2009

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.