Re* ext3: fix ext3_dx_readdir hash collision handling - Regression

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> The "git buglet" looks liek this:
>
>  - build a kernel
>
>  - do "git clean -dqfx". This fails with
>
> 	warning: failed to remove 'include/config/'
>
>  - do "git clean -dqfx" again. And now it works - apparently because the 
>    first invocation deleted enough of the big directory.
>
> Doing an 'strace' to see what is going on, I see:
>
> 	..
> 	getdents(3, /* 132 entries */, 4096)    = 3888
> 	lstat("include/config/sgi", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
> 	open("include/config/sgi", O_RDONLY|O_NONBLOCK|O_DIRECTORY|0x80000) = 4
> 	fstat(4, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
> 	getdents(4, /* 3 entries */, 4096)      = 80
> 	lstat("include/config/sgi/partition.h", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> 	unlink("include/config/sgi/partition.h") = 0
> 	getdents(4, /* 0 entries */, 4096)      = 0
> 	close(4)                                = 0
> 	rmdir("include/config/sgi")             = 0
> 	lstat("include/config/sgi", 0x7fffdc4d2110) = -1 ENOENT (No such file or directory)
> 	close(3)                                = 0
> 	write(2, "warning: failed to remove \'include/config/\'\n", 44) = 44
> 	..
>
> and notice how it does that
>
> 	lstat("include/config/sgi", ..
>
> _twice_. That, in turn (knowing the git implementation), implies that it 
> was returned twice from that first "getdents(3, ...)" call.
>
> So what git clean does is to scan over the readdir() return values, see if 
> it's a file it knows about, try to remove it if not, lstat() every entry, 
> recurse into them if they are directories, and then remove it. If the 
> lstat() fails, "git clean" will fail.

Hmm, interesting.

-- >8 --
Subject: allow readdir(3) to return the same entry twice

When removing a large directory recursively, repeatedly running unlink(2)
on what we read from readdir(3) can cause readdir(3) (or underlying
getdents(2)) to return the same entry twice.  If we have already removed
it, running lstat() on the entry would fail with ENOENT, but there is no
harm if we ignore such an error.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

 dir.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git i/dir.c w/dir.c
index 0131983..293df4e 100644
--- i/dir.c
+++ w/dir.c
@@ -800,8 +800,11 @@ int remove_dir_recursively(struct strbuf *path, int only_empty)
 
 		strbuf_setlen(path, len);
 		strbuf_addstr(path, e->d_name);
-		if (lstat(path->buf, &st))
-			; /* fall thru */
+		if (lstat(path->buf, &st)) {
+			if (errno == ENOENT)
+				continue;
+			/* otherwise fall thru */
+		}
 		else if (S_ISDIR(st.st_mode)) {
 			if (!remove_dir_recursively(path, only_empty))
 				continue; /* happy */
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux