On Fri, 19 Oct 2007, Todd T. Fries wrote:
>
> 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?
Yes. I think this is ok, although I also did this alternate patch that is
anal about not bothering to call "lstat()" if it can decide that the path
is ignored even before that.
That happened in the case of a pathname that was ignored, and we did not
ask for "dir->show_ignored". That test used to be *together* with the
"DTYPE(de) != DT_DIR", but splitting the two tests up means that we can do
that (common) test before we even bother to calculate the real dtype.
Of course, that optimization only matters for systems that don't have, or
don't fill in DTYPE properly, but I get a bit anal about these kinds of
optimizations.
I also clarified the real relationship between "exclude" and
"dir->show_ignored". It used to do
if (exclude != dir->show_ignored) {
..
which wasn't exactly obvious, because it triggers for two different cases:
- the path is marked excluded, but we are not interested in ignored
files: ignore it
- the path is *not* excluded, but we *are* interested in ignored files:
ignore it unless it's a directory, in which case we might have ignored
files inside the directory and need to recurse into it).
so this splits them into those two cases, since the first case doesn't
even care about the type.
I also made a the DT_UNKNOWN case a separate helper function, and added
some commentary to the cases.
Does this patch work for you?
Linus
---
dir.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/dir.c b/dir.c
index eb6c3ab..f843c4d 100644
--- a/dir.c
+++ b/dir.c
@@ -443,6 +443,24 @@ static int in_pathspec(const char *path, int len, const struct path_simplify *si
return 0;
}
+static int get_dtype(struct dirent *de, const char *path)
+{
+ int dtype = DTYPE(de);
+ struct stat st;
+
+ if (dtype != DT_UNKNOWN)
+ return dtype;
+ if (lstat(path, &st))
+ return dtype;
+ if (S_ISREG(st.st_mode))
+ return DT_REG;
+ if (S_ISDIR(st.st_mode))
+ return DT_DIR;
+ if (S_ISLNK(st.st_mode))
+ return DT_LNK;
+ return dtype;
+}
+
/*
* Read a directory tree. We currently ignore anything but
* directories, regular files and symlinks. That's because git
@@ -466,7 +484,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
exclude_stk = push_exclude_per_directory(dir, base, baselen);
while ((de = readdir(fdir)) != NULL) {
- int len;
+ int len, dtype;
int exclude;
if ((de->d_name[0] == '.') &&
@@ -486,24 +504,30 @@ 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 (exclude != dir->show_ignored) {
- if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
+
+ /*
+ * Excluded? If we don't explicitly want to show
+ * ignored files, ignore it
+ */
+ if (exclude && !dir->show_ignored)
+ continue;
+
+ dtype = get_dtype(de, fullname);
+
+ /*
+ * Do we want to see just the ignored files?
+ * We still need to recurse into directories,
+ * even if we don't ignore them, since the
+ * directory may contain files that we do..
+ */
+ if (!exclude && dir->show_ignored) {
+ if (dtype != DT_DIR)
continue;
- }
}
- switch (DTYPE(de)) {
- struct stat st;
+ switch (dtype) {
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))
- continue;
- /* fallthrough */
case DT_DIR:
memcpy(fullname + baselen + len, "/", 2);
len++;
-
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
