Re: [PATCH] Do _not_ call unlink on a directory
> This is wrong. If the filesystem has a symlink and we would want a
> directory there, we should unlink(). So at least the stat there needs
> to be lstat().
> I wonder if anybody involved in the discussion has actually
> tested this patch (or the other one, that has the same problem)?
I tested it. But I did not test it with symlinks.
> Does the following replacement work for you? It adds far more lines
> than your version, but they are mostly comments to make it clear why
> we do things this way.
Yes, it does. Excuse the delay but my build machine is not the fastest.
(faui04a) [/var/tmp] git clone ~/work/repositories/public/easix.git test-10
Initialized empty Git repository in /var/tmp/test-10/.git/
remote: Generating pack...
remote: Done counting 317 objects.
remote: Deltifying 317 objects...
remote: te: % (317/317) done: ) done
Indexing 317 objects...
remote: Total 317 (delta 182), reused 278 (delta 157)
100% (317/317) done
Resolving 182 deltas...
100% (182/182) done
(faui04a) [/var/tmp] cd test-10
./test-10
(faui04a) [/var/tmp/test-10] git status
# On branch master
nothing to commit (working directory clean)
I rebased your patch on top of current HEAD (as I can access it on git.kernel.org) and removed trailing whitspace from one line (git-apply complained)
Thomas
>From 3b60b807007507ce5e1f8490f1469dac5bb95917 Mon Sep 17 00:00:00 2001
From: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>
Date: Tue, 17 Jul 2007 11:31:07 +0200
Subject: [PATCH] Do _not_ call unlink on a directory
Calling unlink on a directory on a Solaris UFS filesystem as root makes it
inconsistent. Thanks to Junio for the not so obvious fix.
---
entry.c | 37 ++++++++++++++++++++++++++++++-------
1 files changed, 30 insertions(+), 7 deletions(-)
Show changes to entry.c +30 −7
diff --git a/entry.c b/entry.c
index c540ae1..0625112 100644
--- a/entry.c
+++ b/entry.c
@@ -8,17 +8,40 @@ static void create_directories(const char *path, const struct checkout *state)
const char *slash = path;
while ((slash = strchr(slash+1, '/')) != NULL) {
+ struct stat st;
+ int stat_status;
+
len = slash - path;
memcpy(buf, path, len);
buf[len] = 0;
+
+ if (len <= state->base_dir_len)
+ /*
+ * checkout-index --prefix=<dir>; <dir> is
+ * allowed to be a symlink to an existing
+ * directory.
+ */
+ stat_status = stat(buf, &st);
+ else
+ /*
+ * if there currently is a symlink, we would
+ * want to replace it with a real directory.
+ */
+ stat_status = lstat(buf, &st);
+
+ if (!stat_status && S_ISDIR(st.st_mode))
+ continue; /* ok, it is already a directory. */
+
+ /*
+ * We know stat_status == 0 means something exists
+ * there and this mkdir would fail, but that is an
+ * error codepath; we do not care, as we unlink and
+ * mkdir again in such a case.
+ */
if (mkdir(buf, 0777)) {
- if (errno == EEXIST) {
- struct stat st;
- if (len > state->base_dir_len && state->force && !unlink(buf) && !mkdir(buf, 0777))
- continue;
- if (!stat(buf, &st) && S_ISDIR(st.st_mode))
- continue; /* ok */
- }
+ if (errno == EEXIST && state->force &&
+ !unlink(buf) && !mkdir(buf, 0777))
+ continue;
die("cannot create directory at %s", buf);
}
}
--
1.5.2.1
