On Tue, May 11, 2021 at 3:43 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Elijah Newren <newren@xxxxxxxxx> writes: > > > But to answer your question, the paths we visit are '.', '..', '.git', > > and 'untracked', the first three of which we mark as path_none and > > don't recurse into because of special rules for those paths, and the > > last of which we shouldn't recurse into since it is ignored. > > Not a hard requirement, but I wish if we entirely ignored "." and > ".." in our code (not just not counting, but making whoever calls > readdir() skip and call it again when it gets "." or ".."). > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/readdir.html > > seems to imply that readdir() may not give "." or ".." (if dot or > dot-dot exists, you are to return them only once, which implies that > it is perfectly OK for dot or dot-dot to be missing). Something like this? diff --git a/dir.c b/dir.c index 993a12145f..7f470bc701 100644 --- a/dir.c +++ b/dir.c @@ -2341,7 +2341,11 @@ static int read_cached_dir(struct cached_dir *cdir) struct dirent *de; if (cdir->fdir) { - de = readdir(cdir->fdir); + while ((de = readdir(cdir->fdir))) { + /* Ignore '.' and '..' by re-looping; handle the rest */ + if (!de || !is_dot_or_dotdot(de->d_name)) + break; + } if (!de) { cdir->d_name = NULL; cdir->d_type = DT_UNKNOWN; It appears that the other two callers of readdir() in dir.c, namely in is_empty_dir() and remove_dir_recurse() already have such special repeat-if-is_dot_or_dotdot() logic built into them, so this was partially lifted from those. If you'd like, I can add another patch in the series with this change so that all readdir() calls in dir.c have such ignore '.' and '..' logic. Or, we could perhaps introduce a new readdir() wrapper that does nothing other than ignore '.' and '..' and have all three of these callsites use that new wrapper. > So dropping the test for number of visited paths would be nicer from > portability's point of view ;-) Yep, makes sense. I already did that in v4, which means it'll continue to pass with or without the above proposed change to read_cached_dir().
