git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: "Pickens, James E" <james.e.pickens@intel.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Kjetil Barvik <barvik@broadpark.no>,
	Michael J Gruber <git@drmicha.warpmail.net>
Subject: Re: [PATCH v3] Demonstrate bugs when a directory is replaced with a symlink
Date: Wed, 29 Jul 2009 15:08:12 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.01.0907291440480.3161@localhost.localdomain> (raw)
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;

  reply	other threads:[~2009-07-29 22:09 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-28 22:13 More symlink/directory troubles James Pickens
2009-07-28 22:13 ` [PATCH 1/2] Demonstrate bugs when a directory is replaced with a symlink James Pickens
2009-07-28 22:13   ` [PATCH 2/2] Demonstrate merge failure " James Pickens
2009-07-29  8:29     ` Michael J Gruber
2009-07-29 16:39       ` Pickens, James E
2009-07-29  8:19   ` [PATCH 1/2] Demonstrate bugs " Michael J Gruber
2009-07-29  8:33     ` Junio C Hamano
2009-07-29 16:57       ` Pickens, James E
2009-07-29 17:48       ` [PATCH v2] " Pickens, James E
2009-07-29 18:31         ` Junio C Hamano
2009-07-29 21:02           ` [PATCH v3] " Pickens, James E
2009-07-29 22:08             ` Linus Torvalds [this message]
2009-07-29 22:44               ` Junio C Hamano
2009-07-29 23:01               ` Kjetil Barvik
2009-07-29 23:58               ` Linus Torvalds
2009-07-30  1:06                 ` Linus Torvalds
2009-07-30  3:26               ` Junio C Hamano
2009-07-30  6:05               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.01.0907291440480.3161@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=barvik@broadpark.no \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=james.e.pickens@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).