Re: [PATCH v3] Demonstrate bugs when a directory is replaced with a symlink

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Jul 29, 2009, 22:08 UTC
Message-ID: <alpine.LFD.2.01.0907291440480.3161@localhost.localdomain>
In-Reply-To: <3BA20DF9B35F384F8B7395B001EC3FB342402D3C@azsmsx507.amr.corp.intel.com>

On Wed, 29 Jul 2009, Pickens, James E wrote:

>
> This test creates two directories, a/b and a/b-2, then replaces a/b with
> a symlink to a/b-2, then merges that change into the 'baseline' commit,
> which contains an unrelated change.

Great tests.

This patch should fix the 'checkout' issue.

I made it use a new generic helper function ("check_path()"), since there are other cases like this that use just 'lstat()', and I bet we want to change that.

The 'merge' issue is different, though: it's not due to a blind 'lstat()', but due to a blind 'unlink()' done by 'remove_path()'. I think 'remove_path()' should be taught to look for symlinks, and remove just the symlink - but that's a bit more work, especially since the symlink cache doesn't seem to expose any way to get the "what is the first symlink path" information.

Kjetil, can you look at that?

		Linus

---
 cache.h |    3 +++
 entry.c |   15 ++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index e6c7f33..9222774 100644
--- a/cache.h
+++ b/cache.h
@@ -468,6 +468,9 @@ extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_obje
 extern int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object);
 extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
 
+/* "careful lstat()" */
+extern int check_path(const char *path, int len, struct stat *st);
+
 #define REFRESH_REALLY		0x0001	/* ignore_valid */
 #define REFRESH_UNMERGED	0x0002	/* allow unmerged */
 #define REFRESH_QUIET		0x0004	/* be quiet about it */
diff --git a/entry.c b/entry.c
index d3e86c7..f276cf3 100644
--- a/entry.c
+++ b/entry.c
@@ -175,6 +175,19 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
 	return 0;
 }
 
+/*
+ * This is like 'lstat()', except it refuses to follow symlinks
+ * in the path.
+ */
+int check_path(const char *path, int len, struct stat *st)
+{
+	if (has_symlink_leading_path(path, len)) {
+		errno = ENOENT;
+		return -1;
+	}
+	return lstat(path, st);
+}
+
 int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath)
 {
 	static char path[PATH_MAX + 1];
@@ -188,7 +201,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
 	strcpy(path + len, ce->name);
 	len += ce_namelen(ce);
 
-	if (!lstat(path, &st)) {
+	if (!check_path(path, len, &st)) {
 		unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID);
 		if (!changed)
 			return 0;