Re: git on afs

["Todd T. Fries" <todd@fries.net>]

[-- Attachment #1: Type: text/plain, Size: 2631 bytes --]

Shawn,

If DT_UNKNOWN exists, then we have to do a stat() of some form to
find out the right type.

This is difficult to fix properly to avoid the extra stat() since
inside the switch logic we do the recursion, but we might have
avoided it earlier because of the exclusion.

I'll send a separate diff for an updated link() vs rename() diff.

I've attached an updated diff that should address concerns of everyone
who gave me feedback on my dir.c changes.

Better?

Thanks,

On Friday 19 October 2007 01:06:24 Shawn O. Pearce wrote:
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > On Thu, 18 Oct 2007, Todd T. Fries wrote:
> > > 2) git presumes that DTYPE(de) != DT_DIR .. means the dirent is not a
> > > dir this is not true for afs
> >
> > That's a major bug, and has nothing to do with AFS. Oops.
> >
> > If you look just a bit lower, you'll see that just a few lines down, git
> > handles DT_UNKNOWN correctly, and just does a lstat() on it as required.
> > I guess that logic should be moved up, or alternatively the exclude logic
> > should be moved down.
> >
> > Your patch looks ok, but at the same time, I don't think it's really the
> > right thing to do, since it now does that lstat() twice.
>
> What about this instead?  It avoids the double lstat() of Todd's
> original patch but seems like it would fix the issue here.  Or did
> I misunderstand the problem?
[..]
> diff --git a/dir.c b/dir.c
> index eb6c3ab..d2597ff 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -487,9 +487,10 @@ static int read_directory_recursive(struct dir_struct
> *dir, const char *path, co && in_pathspec(fullname, baselen + len,
> simplify))
>  				dir_add_ignored(dir, fullname, baselen + len);
>  			if (exclude != dir->show_ignored) {
> -				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
> +				if (!dir->show_ignored)
> +					continue;
> +				else if (DTYPE(de) != DT_DIR && DTYPE(de) != DT_UNKNOWN)
>  					continue;
> -				}
>  			}
>
>  			switch (DTYPE(de)) {
> --
> 1.5.3.4.1249.g895be



-- 
Todd Fries .. todd@fries.net

 _____________________________________________
|                                             \  1.636.410.0632 (voice)
| Free Daemon Consulting                      \  1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com             \  1.866.792.3418 (FAX)
| "..in support of free software solutions."  \          250797 (FWD)
|                                             \
 \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
                                                 
              37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
                        http://todd.fries.net/pgp.txt






[-- Attachment #2: 0001-If-readdir-returns-a-DTYPE-of-DT_UNKNOWN-this-w.patch --]
[-- Type: text/x-diff, Size: 1848 bytes --]

From f7bca472645db70b52824e1be827ba99195198d2 Mon Sep 17 00:00:00 2001
From: Todd T. Fries <todd@fries.net>
Date: Fri, 19 Oct 2007 06:26:49 -0500
Subject: [PATCH] If readdir() returns a DTYPE() of DT_UNKNOWN, this warrants further
investigation, i.e. a stat() or lstat().  We have been presuming
anything not DT_DIR is not a dir, which is not the case for
all filesystems (e.g. afs).

Do the lstat() once, and use the results twice.

Signed-off-by: Todd T. Fries <todd@fries.net>
---
 dir.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/dir.c b/dir.c
index eb6c3ab..0ed4739 100644
--- a/dir.c
+++ b/dir.c
@@ -468,6 +468,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 		while ((de = readdir(fdir)) != NULL) {
 			int len;
 			int exclude;
+			struct stat st;
 
 			if ((de->d_name[0] == '.') &&
 			    (de->d_name[1] == 0 ||
@@ -486,19 +487,24 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			if (exclude && dir->collect_ignored
 			    && in_pathspec(fullname, baselen + len, simplify))
 				dir_add_ignored(dir, fullname, baselen + len);
+			if (DTYPE(de) == DT_UNKNOWN) {
+				if (lstat(fullname, &st))
+					continue;
+			}
 			if (exclude != dir->show_ignored) {
-				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
+				if (!dir->show_ignored))
 					continue;
-				}
+				if (DTYPE(de) == DT_UNKNOWN && !S_ISDIR(st.st_mode))
+					continue;
+				else
+					if (DTYPE(de) != DT_DIR)
+						continue;
 			}
 
 			switch (DTYPE(de)) {
-			struct stat st;
 			default:
 				continue;
 			case DT_UNKNOWN:
-				if (lstat(fullname, &st))
-					continue;
 				if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode))
 					break;
 				if (!S_ISDIR(st.st_mode))
-- 
1.5.2.5