bug-gnulib
[Top][All Lists]
Advanced

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

making fts thread-safe (no more fchdir)


From: Jim Meyering
Subject: making fts thread-safe (no more fchdir)
Date: Tue, 17 Jan 2006 22:33:18 +0100

[ I've checked in a pretty big (conceptually, at least) coreutils change
  today.  Here's some of the background and justification, along with
  the actual patch.  ]

I started my little cwd/thread-safety crusade with GNU rm.  Fixing
the core function to be thread safe meant rewriting remove.c not to
change the current working directory, while keeping it efficient for
very deep hierarchies.

However, note that thread-safety wasn't the only motivation for this work.
A big one was simply to make the code more robust.  The moment a program
changes the working directory, it risks not being able to return to it.
And if it fails to return to the original working directory, it can't
very well continue e.g., trying to remove "."-relative names.

With today's change, I've done the same thing for du, chmod, chown, and
chgrp, all of which rely on fts to perform a hierarchy traversal.
The core of the change is to make fts maintain a file descriptor that is
open on a *virtual* working directory.  So now, rather than using chdir
or fchdir (which would change the per-process current working directory)
to traverse a hierarchy, fts simply advances its virtual cwd-FD to each
new directory.  However, this new functionality requires an API change
and, for best results, some kernel-supported functions named e.g.,
openat, fstatat, fchmodat, etc.


FTS API change:
==============

This changes the fts API.  Before, callers (those not using FTS_NOCHDIR)
could expect fts_read to change the current working directory so that
a simple directory entry name (fts_accpath) could be used to access
files in the directory in question.  Now, the current working directory
is never changed, and instead, each caller must use an openat-like
function to access that same name.  The only difference is that they
must also use the new member, fts_cwd_fd, which is a file descriptor
open on the virtual current working directory.

So, whereas chmod(1) used to do this:

    if (chmod (file, new_mode) == 0)

with the new API, now it does this:

    if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)

In both cases, file is defined like this:

    char const *file = ent->fts_accpath;

For now, this change is only in the coreutils.
I don't know how many other gnulib clients (other than GNU find)
use fts.  If you know of a package that uses gnulib's fts[*], but that
would have a hard time switching to the new API, please let me know.
As you can see below, switching du.c was a no-op, and converting
the other three (chmod, chgrp, chown) was very easy.

[*] If you're using glibc's fts, then you should switch;
it has many limitations and bugs not present in the gnulib version.


Efficiency:
==========

On systems with kernel support for functions like openat, fstatat, etc.,
these changes result in a small performance improvement.  Solaris 9
and 10 provide most of the functions in the kernel.  For Linux, these
new functions will appear soon: look for the at-functions patches on
LKML.  E.g., <http://google.com/search?q=lkml+openat+at-functions>

In the mean time, on systems like Linux with enough PROC_FS support that
gnulib's openat emulation works well, there is a small performance
degradation due to the overhead of building and decoding each
/proc/self/fd/... file name.  There is also some overhead involved in
simulating fdopendir, but that will go away on glibc-based systems once
people start using glibc-2.4, which provides fdopendir.

On systems with neither kernel openat support nor /proc support,
there will be more overhead.  I haven't measured it and confess that
I'm not too concerned.

But we needn't worry much about efficiency.  Most of the effects I've
measured on Linux are insignificant in real usage (either there's so
little work to do that we don't care either way, or these minor changes
are buried under the cost of lots of I/O).  The actual effects can be
measured reliably only when all inodes are already in cache.

If you do care a lot about efficiency and want to do something even on
Linux while waiting for glibc-2.4, let me know: I have a glibc-specific
fdopendir replacement that works -- using it removes about half of the
added overhead.  The caveat is that it uses knowledge of libc internals
and hard-codes things it probably shouldn't.  Writing an autoconf test
to see if this replacement works might prove tricky.

Here are the actual diffs:




 lib/fts.c                     |  217 +++++++++++++++++++++++++++---------------
 lib/fts_.h                    |   14 +-
 m4/fts.m4                     |    3
 src/chmod.c                   |    3
 src/chown-core.c              |   14 +-
 tests/du/long-from-unreadable |   70 +++++++++++++
 6 files changed, 233 insertions(+), 88 deletions(-)


=== coreutils/lib ===
2006-01-17  Jim Meyering  <address@hidden>

        Rewrite fts.c not to change the current working directory,
        by using openat, fstatat, fdopendir, etc..

        * fts.c [! _LIBC]: Include "openat.h" and "unistd--.h".
        (HAVE_OPENAT_SUPPORT): Define.
        [_LIBC] (fchdir): Don't undef or define; no longer used.
        (FCHDIR): Define in terms of cwd_advance_fd rather than fchdir.
        Now, this `function' always succeeds, and consumes its file descriptor
        parameter -- so callers must not close such FDs.  Update callers.
        (diropen_fd, opendirat, cwd_advance_fd): New functions.
        (diropen): Add parameter, SP.  Adjust all callers.
        Implement using diropen_fd, rather than open.
        (fts_open): Initialize new member, fts_cwd_fd.
        Remove fts_rft-setting code.
        (fts_close): Close fts_cwd_fd, if necessary.
        (__opendir2): Define in terms of opendir or opendirat,
        depending on whether the FST_NOCHDIR flag is set.
        (fts_build): Since fts_safe_changedir consumes its FD, and since
        this code must do `closedir(dirp)', dup the dirfd(dirp) argument,
        and close the dup'd file descriptor upon failure.
        (fts_stat): Use fstatat(...AT_SYMLINK_NOFOLLOW) in place of lstat.
        (fts_safe_changedir): Tweak semantics to reflect that this function
        now calls cwd_advance_fd and hence consumes its FD argument.
        * fts_.h [struct FTS] (fts_cwd_fd): New member.
        [struct FTS] (fts_rft): Remove now-unused member.
        [struct FTS] (fts_cycle.state): Improve comment.


=== coreutils ===
2006-01-17  Jim Meyering  <address@hidden>

        * Version 6.0-cvs.

        Now that fts no longer changes the current working directory, adjust
        its clients accordingly -- note that du.c uses fts but doesn't need
        any adjustment, since it doesn't operate on the actual files,
        but rather just uses the stat buffers provided by fts.

        * src/chown-core.c: Include "openat.h".
        Don't include "lchown.h".
        (restricted_chown): Accept a new parameter, CWD_FD, and use it in
        calling openat, lchownat, chownat, rather than open, lchown, chown.
        Update caller.
        * src/chmod.c: Include "openat.h".
        (process_file): Use chmodat (fts->fts_cwd_fd,... in place of chmod (...

        * tests/du/long-from-unreadable: New test, to exercise one small
        corner of fts.c.

=== coreutils/m4 ===
2006-01-17  Jim Meyering  <address@hidden>

        * fts.m4 (gl_FUNC_FTS_CORE): Depend on gl_FUNC_OPENAT.


Index: lib/fts.c
===================================================================
RCS file: /fetish/cu/lib/fts.c,v
retrieving revision 1.43
diff -u -p -r1.43 fts.c
--- lib/fts.c   11 Jan 2006 21:00:36 -0000      1.43
+++ lib/fts.c   17 Jan 2006 17:19:19 -0000
@@ -72,8 +72,10 @@ static char sccsid[] = "@(#)fts.c    8.6 (B
 #include <unistd.h>
 
 #if ! _LIBC
-# include "lstat.h"
 # include "fcntl--.h"
+# include "lstat.h"
+# include "openat.h"
+# include "unistd--.h"
 #endif
 
 #if defined _LIBC
@@ -103,8 +105,6 @@ static char sccsid[] = "@(#)fts.c   8.6 (B
 # define close __close
 # undef closedir
 # define closedir __closedir
-# undef fchdir
-# define fchdir __fchdir
 # undef open
 # define open __open
 # undef opendir
@@ -130,6 +130,13 @@ static char sccsid[] = "@(#)fts.c  8.6 (B
 # define ATTRIBUTE_UNUSED __attribute__ ((__unused__))
 #endif
 
+/* If this host provides the openat function, then we can avoid
+   attempting to open "." in some initialization code below.  */
+#ifdef HAVE_OPENAT
+# define HAVE_OPENAT_SUPPORT 1
+#else
+# define HAVE_OPENAT_SUPPORT 0
+#endif
 
 static FTSENT  *fts_alloc (FTS *, const char *, size_t) internal_function;
 static FTSENT  *fts_build (FTS *, int) internal_function;
@@ -171,7 +178,9 @@ static void free_dir (FTS *fts) {}
 #define ISSET(opt)     (sp->fts_options & (opt))
 #define SET(opt)       (sp->fts_options |= (opt))
 
-#define FCHDIR(sp, fd) (!ISSET(FTS_NOCHDIR) && fchdir(fd))
+#define FCHDIR(sp, fd) (!ISSET(FTS_NOCHDIR) \
+                        && (cwd_advance_fd (sp, fd), 0))
+
 
 /* fts_build flags */
 #define BCHILD         1               /* fts_children */
@@ -196,15 +205,64 @@ bool fts_debug = false;
     }                                                          \
   while (false)
 
+/* Open the directory DIR if possible, and return a file descriptor.
+   As with openat-like functions, if DIR is a relative name,
+   interpret it relative to the directory open on file descriptor FD.
+   Return -1 and set errno on failure.  */
+static int
+internal_function
+diropen_fd (int cwd_fd, char const *dir)
+{
+  int fd = openat (cwd_fd, dir,
+                  O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
+  return fd;
+}
+
+/* file-descriptor-relative opendir.  */
+/* FIXME: if others need this function, move it into lib/openat.c */
+static inline DIR *
+internal_function
+opendirat (int fd, char const *dir)
+{
+  int new_fd = openat (fd, dir, O_RDONLY);
+  DIR *dirp;
+
+  if (new_fd < 0)
+    return NULL;
+  dirp = fdopendir (new_fd);
+  if (dirp == NULL)
+    {
+      int saved_errno = errno;
+      close (new_fd);
+      errno = saved_errno;
+    }
+  return dirp;
+}
+
+/* Virtual fchdir.  Advance SP's working directory
+   file descriptor, SP->fts_cwd_fd, to FD, and close
+   the previous one, ignoring any error.  */
+static void
+internal_function
+cwd_advance_fd (FTS *sp, int fd)
+{
+  int old = sp->fts_cwd_fd;
+  if (old == fd && old != AT_FDCWD)
+    abort ();
+  sp->fts_cwd_fd = fd;
+  if (0 <= old)
+    close (old); /* ignore any close failure */
+}
+
 /* 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.  */
 
-static int
+static inline int
 internal_function
-diropen (char const *dir)
+diropen (FTS const *sp, char const *dir)
 {
-  return open (dir, O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
+  return diropen_fd (sp->fts_cwd_fd, dir);
 }
 
 FTS *
@@ -235,6 +293,40 @@ fts_open (char * const *argv,
        if (ISSET(FTS_LOGICAL))
                SET(FTS_NOCHDIR);
 
+       /* Initialize fts_cwd_fd.  */
+       sp->fts_cwd_fd = AT_FDCWD;
+       if ( ! ISSET(FTS_NOCHDIR) && ! HAVE_OPENAT_SUPPORT)
+         {
+           /* While it isn't technically necessary to open "." this
+              early, doing it here saves us the trouble of ensuring
+              later (where it'd be messier) that "." can in fact
+              be opened.  If not, revert to FTS_NOCHDIR mode.  */
+           int fd = open (".", O_RDONLY);
+           if (fd < 0)
+             {
+               /* Even if `.' is unreadable, don't revert to FTS_NOCHDIR mode
+                  on systems like Linux+PROC_FS, where our openat emulation
+                  is good enough.  Note: on a system that emulates
+                  openat via /proc, this technique can still fail, but
+                  only in extreme conditions, e.g., when the working
+                  directory cannot be saved (i.e. save_cwd fails) --
+                  and that happens only on Linux only when "." is unreadable
+                  and the CWD would be longer than PATH_MAX.
+                  FIXME: once Linux kernel openat support is well established,
+                  replace the above open call and this entire if/else block
+                  with the body of the if-block below.  */
+               if ( openat_needs_fchdir ())
+                 {
+                   SET(FTS_NOCHDIR);
+                   sp->fts_cwd_fd = -1;
+                 }
+             }
+           else
+             {
+               close (fd);
+             }
+         }
+
        /*
         * Start out with 1K of file name space, and enough, in any case,
         * to hold the user's file names.
@@ -304,17 +396,6 @@ fts_open (char * const *argv,
        if (! setup_dir (sp))
          goto mem3;
 
-       /*
-        * If using chdir(2), grab a file descriptor pointing to dot to ensure
-        * that we can get back here; this could be avoided for some file names,
-        * but almost certainly not worth the effort.  Slashes, symbolic links,
-        * and ".." are all fairly nasty problems.  Note, if we can't get the
-        * descriptor we run anyway, just more slowly.
-        */
-       if (!ISSET(FTS_NOCHDIR)
-           && (sp->fts_rfd = diropen (".")) < 0)
-               SET(FTS_NOCHDIR);
-
        return (sp);
 
 mem3:  fts_lfree(root);
@@ -376,12 +457,8 @@ fts_close (FTS *sp)
                free(sp->fts_array);
        free(sp->fts_path);
 
-       /* Return to original directory, save errno if necessary. */
-       if (!ISSET(FTS_NOCHDIR)) {
-               if (fchdir(sp->fts_rfd))
-                       saved_errno = errno;
-               (void)close(sp->fts_rfd);
-       }
+       if (0 <= sp->fts_cwd_fd)
+         close (sp->fts_cwd_fd);
 
        free_dir (sp);
 
@@ -411,7 +488,6 @@ fts_read (register FTS *sp)
        register FTSENT *p, *tmp;
        register unsigned short int instr;
        register char *t;
-       int saved_errno;
 
        /* If finished or unrecoverable error, return NULL. */
        if (sp->fts_cur == NULL || ISSET(FTS_STOP))
@@ -442,7 +518,7 @@ fts_read (register FTS *sp)
            (p->fts_info == FTS_SL || p->fts_info == FTS_SLNONE)) {
                p->fts_info = fts_stat(sp, p, true);
                if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) {
-                       if ((p->fts_symfd = diropen (".")) < 0) {
+                       if ((p->fts_symfd = diropen (sp, ".")) < 0) {
                                p->fts_errno = errno;
                                p->fts_info = FTS_ERR;
                        } else
@@ -503,7 +579,6 @@ fts_read (register FTS *sp)
                           subdirectory, tell the caller.  */
                        if (p->fts_errno)
                                p->fts_info = FTS_ERR;
-                       /* FIXME: see if this should be in an else block */
                        LEAVE_DIR (sp, p, "2");
                        return (p);
                }
@@ -523,11 +598,7 @@ next:      tmp = p;
                 * root.
                 */
                if (p->fts_level == FTS_ROOTLEVEL) {
-                       if (FCHDIR(sp, sp->fts_rfd)) {
-                               SET(FTS_STOP);
-                               sp->fts_cur = p;
-                               return (NULL);
-                       }
+                       FCHDIR(sp, AT_FDCWD);
                        fts_load(sp, p);
                        goto check_for_dir;
                }
@@ -542,7 +613,7 @@ next:       tmp = p;
                if (p->fts_instr == FTS_FOLLOW) {
                        p->fts_info = fts_stat(sp, p, true);
                        if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) {
-                               if ((p->fts_symfd = diropen (".")) < 0) {
+                               if ((p->fts_symfd = diropen (sp, ".")) < 0) {
                                        p->fts_errno = errno;
                                        p->fts_info = FTS_ERR;
                                } else
@@ -586,23 +657,15 @@ check_for_dir:
        sp->fts_path[p->fts_pathlen] = '\0';
 
        /*
-        * Return to the parent directory.  If at a root node or came through
-        * a symlink, go back through the file descriptor.  Otherwise, cd up
-        * one directory.
+        * Return to the parent directory.  If at a root node, just set
+        * sp->fts_cwd_fd to AT_FDCWD.  If we came through a symlink,
+        * go back through the file descriptor.  Otherwise, move up
+        * one level, via "..".
         */
        if (p->fts_level == FTS_ROOTLEVEL) {
-               if (FCHDIR(sp, sp->fts_rfd)) {
-                       p->fts_errno = errno;
-                       SET(FTS_STOP);
-               }
+               FCHDIR(sp, AT_FDCWD);
        } else if (p->fts_flags & FTS_SYMFOLLOW) {
-               if (FCHDIR(sp, p->fts_symfd)) {
-                       saved_errno = errno;
-                       (void)close(p->fts_symfd);
-                       __set_errno (saved_errno);
-                       p->fts_errno = errno;
-                       SET(FTS_STOP);
-               }
+               FCHDIR(sp, p->fts_symfd);
                (void)close(p->fts_symfd);
        } else if (!(p->fts_flags & FTS_DONTCHDIR) &&
                   fts_safe_changedir(sp, p->fts_parent, -1, "..")) {
@@ -692,14 +755,10 @@ fts_children (register FTS *sp, int inst
            ISSET(FTS_NOCHDIR))
                return (sp->fts_child = fts_build(sp, instr));
 
-       if ((fd = diropen (".")) < 0)
+       if ((fd = diropen (sp, ".")) < 0)
                return (sp->fts_child = NULL);
        sp->fts_child = fts_build(sp, instr);
-       if (fchdir(fd)) {
-               (void)close(fd);
-               return (NULL);
-       }
-       (void)close(fd);
+       cwd_advance_fd (sp, fd);
        return (sp->fts_child);
 }
 
@@ -750,7 +809,10 @@ fts_build (register FTS *sp, int type)
        else
                oflag = DTF_HIDEW|DTF_NODUP|DTF_REWIND;
 #else
-# define __opendir2(file, flag) opendir(file)
+# define __opendir2(file, flag) \
+       (ISSET(FTS_NOCHDIR) \
+        ? opendir(file) \
+        : opendirat(sp->fts_cwd_fd, file))
 #endif
        if ((dirp = __opendir2(cur->fts_accpath, oflag)) == NULL) {
                if (type == BREAD) {
@@ -795,13 +857,16 @@ fts_build (register FTS *sp, int type)
         */
        cderrno = 0;
        if (nlinks || type == BREAD) {
-               if (fts_safe_changedir(sp, cur, dirfd(dirp), NULL)) {
+               int dir_fd = dup (dirfd(dirp));
+               if (dir_fd < 0 || fts_safe_changedir(sp, cur, dir_fd, NULL)) {
                        if (nlinks && type == BREAD)
                                cur->fts_errno = errno;
                        cur->fts_flags |= FTS_DONTCHDIR;
                        descend = false;
                        cderrno = errno;
                        closedir(dirp);
+                       if (0 <= dir_fd)
+                               close (dir_fd);
                        dirp = NULL;
                } else
                        descend = true;
@@ -962,7 +1027,7 @@ mem1:                              saved_errno = errno;
         */
        if (descend && (type == BCHILD || !nitems) &&
            (cur->fts_level == FTS_ROOTLEVEL ?
-            FCHDIR(sp, sp->fts_rfd) :
+            FCHDIR(sp, AT_FDCWD) :
             fts_safe_changedir(sp, cur->fts_parent, -1, ".."))) {
                cur->fts_info = FTS_ERR;
                SET(FTS_STOP);
@@ -1077,7 +1142,8 @@ fts_stat(FTS *sp, register FTSENT *p, bo
                        p->fts_errno = saved_errno;
                        goto err;
                }
-       } else if (lstat(p->fts_accpath, sbp)) {
+       } else if (fstatat(sp->fts_cwd_fd, p->fts_accpath, sbp,
+                          AT_SYMLINK_NOFOLLOW)) {
                p->fts_errno = errno;
 err:           memset(sbp, 0, sizeof(struct stat));
                return (FTS_NS);
@@ -1305,34 +1371,39 @@ fts_maxarglen (char * const *argv)
  * Change to dir specified by fd or file name without getting
  * tricked by someone changing the world out from underneath us.
  * Assumes p->fts_statp->st_dev and p->fts_statp->st_ino are filled in.
+ * If FD is non-negative, expect it to be used after this function returns,
+ * and to be closed eventually.  So don't pass e.g., `dirfd(dirp)' and then
+ * do closedir(dirp), because that would invalidate the saved FD.
+ * Upon failure, close FD immediately and return nonzero.
  */
 static int
 internal_function
 fts_safe_changedir (FTS *sp, FTSENT *p, int fd, char const *dir)
 {
-       int ret, oerrno, newfd;
        struct stat sb;
-
-       newfd = fd;
-       if (ISSET(FTS_NOCHDIR))
+       int newfd = fd;
+       if (ISSET(FTS_NOCHDIR)) {
+               if (0 <= fd)
+                       close (fd);
                return (0);
-       if (fd < 0 && (newfd = diropen (dir)) < 0)
+       }
+       if (fd < 0 && (newfd = diropen (sp, dir)) < 0)
                return (-1);
        if (fstat(newfd, &sb)) {
-               ret = -1;
-               goto bail;
+               if (0 <= fd) {
+                       int saved_errno = errno;
+                       close (fd);
+                       errno = saved_errno;
+               }
+               return -1;
        }
        if (p->fts_statp->st_dev != sb.st_dev
            || p->fts_statp->st_ino != sb.st_ino) {
+               if (0 <= fd)
+                       close (fd);
                __set_errno (ENOENT);           /* disinformation */
-               ret = -1;
-               goto bail;
+               return -1;
        }
-       ret = fchdir(newfd);
-bail:
-       oerrno = errno;
-       if (fd < 0)
-               (void)close(newfd);
-       __set_errno (oerrno);
-       return (ret);
+       cwd_advance_fd (sp, newfd);
+       return 0;
 }
Index: lib/fts_.h
===================================================================
RCS file: /fetish/cu/lib/fts_.h,v
retrieving revision 1.18
diff -u -p -r1.18 fts_.h
--- lib/fts_.h  12 Aug 2005 13:02:17 -0000      1.18
+++ lib/fts_.h  13 Jan 2006 09:13:55 -0000
@@ -1,6 +1,6 @@
 /* Traverse a file hierarchy.
 
-   Copyright (C) 2004, 2005 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2005, 2006 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -72,9 +72,10 @@ typedef struct {
        struct _ftsent **fts_array;     /* sort array */
        dev_t fts_dev;                  /* starting device # */
        char *fts_path;                 /* file name for this descent */
-       int fts_rfd;                    /* fd for root */
+       int fts_cwd_fd;                 /* the file descriptor on which the
+                                          virtual cwd is open, or AT_FDCWD */
        size_t fts_pathlen;             /* sizeof(path) */
-       size_t fts_nitems;                      /* elements in the sort array */
+       size_t fts_nitems;              /* elements in the sort array */
        int (*fts_compar) (struct _ftsent const **, struct _ftsent const **);
                                        /* compare fn */
 
@@ -134,9 +135,10 @@ typedef struct {
                   of thousands.  */
                struct hash_table *ht;
 
-               /* This data structure uses lazy checking, as done by rm via
-                  cycle-check.c.  It's the default, but it's not appropriate
-                  for programs like du.  */
+               /* FIXME: rename these two members to have the fts_ prefix */
+               /* This data structure uses a lazy cycle-detection algorithm,
+                  as done by rm via cycle-check.c.  It's the default,
+                  but it's not appropriate for programs like du.  */
                struct cycle_check_state *state;
        } fts_cycle;
 # endif
Index: m4/fts.m4
===================================================================
RCS file: /fetish/cu/m4/fts.m4,v
retrieving revision 1.9
diff -u -p -r1.9 fts.m4
--- m4/fts.m4   22 Sep 2005 06:05:39 -0000      1.9
+++ m4/fts.m4   13 Jan 2006 14:40:49 -0000
@@ -1,5 +1,5 @@
 #serial 6
-dnl Copyright (C) 2005 Free Software Foundation, Inc.
+dnl Copyright (C) 2005, 2006 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
 dnl with or without modifications, as long as this notice is preserved.
@@ -27,6 +27,7 @@ AC_DEFUN([gl_FUNC_FTS_CORE],
   AC_LIBOBJ([fts])
 
   dnl Prerequisites of lib/fts.c.
+  AC_REQUIRE([gl_FUNC_OPENAT])
 
   # Checks for header files.
   AC_REQUIRE([AC_HEADER_DIRENT])
Index: src/chown-core.c
===================================================================
RCS file: /fetish/cu/src/chown-core.c,v
retrieving revision 1.39
diff -u -p -r1.39 chown-core.c
--- src/chown-core.c    3 Jan 2006 06:20:06 -0000       1.39
+++ src/chown-core.c    3 Jan 2006 07:14:19 -0000
@@ -27,7 +27,7 @@
 #include "chown-core.h"
 #include "error.h"
 #include "inttostr.h"
-#include "lchown.h"
+#include "openat.h"
 #include "quote.h"
 #include "root-dev-ino.h"
 #include "xfts.h"
@@ -180,7 +180,7 @@ describe_change (const char *file, enum 
    Return one of the RCH_status values.  */
 
 static enum RCH_status
-restricted_chown (char const *file,
+restricted_chown (int cwd_fd, char const *file,
                  struct stat const *orig_st,
                  uid_t uid, gid_t gid,
                  uid_t required_uid, gid_t required_gid)
@@ -201,10 +201,10 @@ restricted_chown (char const *file,
        return RC_do_ordinary_chown;
     }
 
-  fd = open (file, O_RDONLY | open_flags);
+  fd = openat (cwd_fd, file, O_RDONLY | open_flags);
   if (! (0 <= fd
         || (errno == EACCES && S_ISREG (orig_st->st_mode)
-            && 0 <= (fd = open (file, O_WRONLY | open_flags)))))
+            && 0 <= (fd = openat (cwd_fd, file, O_WRONLY | open_flags)))))
     return (errno == EACCES ? RC_do_ordinary_chown : RC_error);
 
   if (fstat (fd, &st) != 0)
@@ -332,7 +332,7 @@ change_file_owner (FTS *fts, FTSENT *ent
     {
       if ( ! chopt->affect_symlink_referent)
        {
-         ok = (lchown (file, uid, gid) == 0);
+         ok = (lchownat (fts->fts_cwd_fd, file, uid, gid) == 0);
 
          /* Ignore any error due to lack of support; POSIX requires
             this behavior for top-level symbolic links with -h, and
@@ -356,7 +356,7 @@ change_file_owner (FTS *fts, FTSENT *ent
             that can be opened, this race condition can be avoided safely.  */
 
          enum RCH_status err
-           = restricted_chown (file, file_stats, uid, gid,
+           = restricted_chown (fts->fts_cwd_fd, file, file_stats, uid, gid,
                                required_uid, required_gid);
          switch (err)
            {
@@ -364,7 +364,7 @@ change_file_owner (FTS *fts, FTSENT *ent
              break;
 
            case RC_do_ordinary_chown:
-             ok = (chown (file, uid, gid) == 0);
+             ok = (chownat (fts->fts_cwd_fd, file, uid, gid) == 0);
              break;
 
            case RC_error:
Index: src/chmod.c
===================================================================
RCS file: /fetish/cu/src/chmod.c,v
retrieving revision 1.113
diff -u -p -r1.113 chmod.c
--- src/chmod.c 29 Jun 2005 16:27:37 -0000      1.113
+++ src/chmod.c 22 Dec 2005 17:19:23 -0000
@@ -28,6 +28,7 @@
 #include "error.h"
 #include "filemode.h"
 #include "modechange.h"
+#include "openat.h"
 #include "quote.h"
 #include "quotearg.h"
 #include "root-dev-ino.h"
@@ -225,7 +226,7 @@ process_file (FTS *fts, FTSENT *ent)
 
       if (! S_ISLNK (old_mode))
        {
-         if (chmod (file, new_mode) == 0)
+         if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
            chmod_succeeded = true;
          else
            {
Index: tests/du/long-from-unreadable
===================================================================
RCS file: tests/du/long-from-unreadable
diff -N tests/du/long-from-unreadable
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/du/long-from-unreadable       17 Jan 2006 16:08:08 -0000
@@ -0,0 +1,70 @@
+#!/bin/sh
+# Show that fts (hence du, chmod, chgrp, chown) fails when all of the
+# following are true:
+#   - `.' is not readable
+#   - operating on a hierarchy containing a relative name longer than PATH_MAX
+#   - run on a system where gnulib's openat emulation must resort to using
+#       save_cwd and restore_cwd (which fail if `.' is not readable).
+# Thus, the following du invocation should succeed on newer Linux and
+# Solaris systems, yet it must fail on systems lacking both openat and
+# /proc support.  However, before coreutils-6.0 this test would fail even
+# on Linux+PROC_FS systems because its fts implementation would revert
+# unnecessarily to using FTS_NOCHDIR mode in this corner case.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  du --version
+fi
+
+. $srcdir/../envvar-check
+
+proc_file=/proc/self/fd
+if ! test -d $proc_file; then
+  cat <<EOF >&2
+$0: Skipping this test.
+It would fail, since your system lacks /proc support.
+EOF
+  (exit 77); exit 77
+fi
+
+pwd=`pwd`
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
+trap 'status=$?; cd $pwd; chmod -R u+rwx $t0; rm -rf $t0 && exit $status' 0
+trap '(exit $?); exit $?' 1 2 13 15
+
+framework_failure=0
+mkdir -p $tmp || framework_failure=1
+cd $tmp || framework_failure=1
+
+dir=`printf %200s ' '|sed 's/ /x/g'`
+
+# Construct a hierarchy containing a relative file with a name
+# longer than PATH_MAX.
+# for i in `seq 52`; do
+#   mkdir $dir || framework_failure=1
+#   cd $dir || framework_failure=1
+# done
+# cd $tmp || framework_failure=1
+
+# Sheesh.  Bash 3.1.5 can't create this hierarchy.  I get
+# cd: error retrieving current directory: getcwd: cannot access parent 
directories:
+# Use perl instead:
+: ${PERL=perl}
+$PERL \
+    -e 'my $d = '$dir'; foreach my $i (1..52)' \
+    -e '  { mkdir ($d, 0700) && chdir $d or die "$!" }' \
+  || framework_failure=1
+
+mkdir inaccessible || framework_failure=1
+cd inaccessible || framework_failure=1
+chmod 0 . || framework_failure=1
+
+if test $framework_failure = 1; then
+  echo "$0: failure in testing framework" 1>&2
+  (exit 1); exit 1
+fi
+
+fail=0
+du -s $pwd/$tmp/$dir > /dev/null || fail=1
+
+(exit $fail); exit $fail




reply via email to

[Prev in Thread] Current Thread [Next in Thread]