# HG changeset patch
# User Jim Meyering <meyering@redhat.com>
# Date 1319402545 -7200
# Node ID cd7ac59d8eb552ac4c48deb3948945ca78838318
# Parent  176cc63d93cb7241a0af5b1600739eeb250acb4c
fts: close parent dir FD before returning from post-traversal fts_read

The problem: the fts-using "mkdir -p A/B; rm -rf A" would attempt to
unlink A, even though an FD open on A remained.  This is suboptimal
(holding a file descriptor open longer than needed), but otherwise not
a problem on Unix-like kernels.  However, on Cygwin with certain Novell
file systems, (see http://cygwin.com/ml/cygwin/2011-10/msg00365.html),
that represents a real problem: it causes the removal of A to fail
with e.g., "rm: cannot remove `A': Device or resource busy"

fts visits each directory twice and keeps a cache (fts_fd_ring) of
directory file descriptors.  After completing the final, FTS_DP,
visit of a directory, RESTORE_INITIAL_CWD intended to clear the FD
cache, but then proceeded to add a new FD to it via the subsequent
FCHDIR (which calls cwd_advance_fd and i_ring_push).  Before, the
final file descriptor would be closed only via fts_close's call to
fd_ring_clear.  Now, it is usually closed earlier, via the final
FTS_DP-returning fts_read call.
* lib/fts.c (restore_initial_cwd): New function, converted from
the macro.  Call fd_ring_clear *after* FCHDIR, not before it.
Update callers.
Reported by Franz Sirl via the above URL, with analysis by Eric Blake
in http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739

diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,28 @@
+2011-10-23  Jim Meyering  <meyering@redhat.com>
+
+	fts: close parent dir FD before returning from post-traversal fts_read
+	The problem: the fts-using "mkdir -p A/B; rm -rf A" would attempt to
+	unlink A, even though an FD open on A remained.  This is suboptimal
+	(holding a file descriptor open longer than needed), but otherwise not
+	a problem on Unix-like kernels.  However, on Cygwin with certain Novell
+	file systems, (see http://cygwin.com/ml/cygwin/2011-10/msg00365.html),
+	that represents a real problem: it causes the removal of A to fail
+	with e.g., "rm: cannot remove `A': Device or resource busy"
+
+	fts visits each directory twice and keeps a cache (fts_fd_ring) of
+	directory file descriptors.  After completing the final, FTS_DP,
+	visit of a directory, RESTORE_INITIAL_CWD intended to clear the FD
+	cache, but then proceeded to add a new FD to it via the subsequent
+	FCHDIR (which calls cwd_advance_fd and i_ring_push).  Before, the
+	final file descriptor would be closed only via fts_close's call to
+	fd_ring_clear.  Now, it is usually closed earlier, via the final
+	FTS_DP-returning fts_read call.
+	* lib/fts.c (restore_initial_cwd): New function, converted from
+	the macro.  Call fd_ring_clear *after* FCHDIR, not before it.
+	Update callers.
+	Reported by Franz Sirl via the above URL, with analysis by Eric Blake
+	in http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739
+
 2011-10-23  Gary V. Vaughan  <gary@gnu.org>
 	    Bruno Haible  <bruno@clisp.org>
 	    Jim Meyering  <jim@meyering.net>
diff --git a/lib/fts.c b/lib/fts.c
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -229,11 +229,6 @@
 #define ISSET(opt)      (sp->fts_options & (opt))
 #define SET(opt)        (sp->fts_options |= (opt))
 
-/* FIXME: make this a function */
-#define RESTORE_INITIAL_CWD(sp)                 \
-  (fd_ring_clear (&((sp)->fts_fd_ring)),        \
-   FCHDIR ((sp), (ISSET (FTS_CWDFD) ? AT_FDCWD : (sp)->fts_rfd)))
-
 /* FIXME: FTS_NOCHDIR is now misnamed.
    Call it FTS_USE_FULL_RELATIVE_FILE_NAMES instead. */
 #define FCHDIR(sp, fd)                                  \
@@ -349,6 +344,18 @@
   sp->fts_cwd_fd = fd;
 }
 
+/* Restore the initial, pre-traversal, "working directory".
+   In FTS_CWDFD mode, we merely call cwd_advance_fd, otherwise,
+   we may actually change the working directory.
+   Return 0 upon success. Upon failure, set errno and return nonzero.  */
+static int
+restore_initial_cwd (FTS *sp)
+{
+  int fail = FCHDIR (sp, ISSET (FTS_CWDFD) ? AT_FDCWD : sp->fts_rfd);
+  fd_ring_clear (&(sp->fts_fd_ring));
+  return fail;
+}
+
 /* Open the directory DIR if possible, and return a file
    descriptor.  Return -1 and set errno on failure.  It doesn't matter
    whether the file descriptor has read or write access.  */
@@ -948,7 +955,7 @@
                  * root.
                  */
                 if (p->fts_level == FTS_ROOTLEVEL) {
-                        if (RESTORE_INITIAL_CWD(sp)) {
+                        if (restore_initial_cwd(sp)) {
                                 SET(FTS_STOP);
                                 return (NULL);
                         }
@@ -1055,7 +1062,7 @@
          * one level, via "..".
          */
         if (p->fts_level == FTS_ROOTLEVEL) {
-                if (RESTORE_INITIAL_CWD(sp)) {
+                if (restore_initial_cwd(sp)) {
                         p->fts_errno = errno;
                         SET(FTS_STOP);
                 }
@@ -1579,7 +1586,7 @@
          */
         if (!continue_readdir && descend && (type == BCHILD || !nitems) &&
             (cur->fts_level == FTS_ROOTLEVEL
-             ? RESTORE_INITIAL_CWD(sp)
+             ? restore_initial_cwd(sp)
              : fts_safe_changedir(sp, cur->fts_parent, -1, ".."))) {
                 cur->fts_info = FTS_ERR;
                 SET(FTS_STOP);