Re: rm (remove.c): Rewrite to use fts: request for review

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 9/3/2009 10:51 PM:
> I thought I'd push this now to make it easier to review.
> 
>>     http://git.sv.gnu.org/cgit/coreutils.git/log/?h=next

Well, it would help if I'd actually attach the right patch.  For the
record of what you've already looked at... (it was late for me last night)

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkqg9mgACgkQ84KuGfSFAYDFIACg0SeaW3oFz7ihf4Ybqq82mrce
QMEAoIss9n2WGM7n1uBcD57Sl6klNvQB
=dtrT
-----END PGP SIGNATURE-----

>From 59e05b929498cc88667f08260821e553f691b8b1 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 3 Sep 2009 21:59:54 -0600
Subject: [PATCH 1/3] rm: use gnulib faccessat

* gnulib: Update to latest gnulib.
* bootstrap.conf (gnulib_modules): Add faccessat.  Replace strdup
with strdup-posix.
* m4/jm-macros.m4 (coreutils_MACROS): Revert previous change, now
that gnulib does it for us.
* src/remove.c (write_protected_non_symlink): Use faccessat in
more situations.
---
 bootstrap.conf  |    3 ++-
 gnulib          |    2 +-
 m4/jm-macros.m4 |    3 ---
 src/remove.c    |    9 ++++-----
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 321029d..1001b0b 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -78,6 +78,7 @@ gnulib_modules="
   euidaccess
   exclude
   exitfail
+  faccessat
   fchdir
   fcntl
   fcntl-safer
@@ -196,7 +197,7 @@ gnulib_modules="
   stdlib-safer
   stpcpy
   stpncpy
-  strdup
+  strdup-posix
   strftime
   strpbrk
   strtoimax
diff --git a/gnulib b/gnulib
index 9b740ea..973333d 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 9b740ea4bcb104eeceb0c847a376d2b7dbb77d1b
+Subproject commit 973333dc422903c68f8a34c9612ef583ed0cb6ff
diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4
index 934d4ed..416a0af 100644
--- a/m4/jm-macros.m4
+++ b/m4/jm-macros.m4
@@ -92,9 +92,6 @@ AC_DEFUN([coreutils_MACROS],
   # for cp.c
   AC_CHECK_FUNCS_ONCE([utimensat])

-  # for remove.c
-  AC_CHECK_FUNCS_ONCE([faccessat])
-
   dnl This can't use AC_REQUIRE; I'm not quite sure why.
   cu_PREREQ_STAT_PROG

diff --git a/src/remove.c b/src/remove.c
index 2db3859..d6d6803 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -172,12 +172,11 @@ write_protected_non_symlink (int fd_cwd,
         mess up with long file names). */

   {
-    /* Use faccessat if possible, so as to avoid the expense
-       of processing an N-component name.  */
-#if HAVE_FACCESSAT && AT_EACCESS
-    if (faccessat (fd_cwd, file, W_OK, AT_EACCESS) == 0)
+    /* This implements #1: on decent systems, either faccessat is
+       native or /proc/self/fd allows us to skip a chdir.  */
+    if (!openat_needs_fchdir ()
+        && faccessat (fd_cwd, file, W_OK, AT_EACCESS) == 0)
       return 0;
-#endif

     /* This implements #5: */
     size_t file_name_len = strlen (full_name);
-- 
1.6.3.3.334.g916e1


>From 891c76bcb6c404a7d090a9b4e791a5a9ec24fe21 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 3 Sep 2009 22:07:39 -0600
Subject: [PATCH 2/3] euidaccess-stat: remove unnecessary macros

* lib/euidaccess-stat.c (F_OK, R_OK, W_OK, X_OK): Delete; now
guaranteed by gnulib.
---
 lib/euidaccess-stat.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/lib/euidaccess-stat.c b/lib/euidaccess-stat.c
index 98ce281..3d0897b 100644
--- a/lib/euidaccess-stat.c
+++ b/lib/euidaccess-stat.c
@@ -25,13 +25,6 @@

 #include <unistd.h>

-#ifndef F_OK
-# define F_OK 0
-# define X_OK 1
-# define W_OK 2
-# define R_OK 4
-#endif
-
 #include "group-member.h"
 #include "stat-macros.h"

-- 
1.6.3.3.334.g916e1


>From 08f7ede5c66c1bda27b9ebc23b9ce6c0256e3044 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 3 Sep 2009 22:45:36 -0600
Subject: [PATCH 3/3] rm: avoid compiler warning

* src/remove.c (rm_fts): Don't allow fall-through when assertions
are disabled.
---
 gnulib       |    2 +-
 src/remove.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnulib b/gnulib
index 973333d..2847b62 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 973333dc422903c68f8a34c9612ef583ed0cb6ff
+Subproject commit 2847b62110d877f18a65bbbb91d406e064fefff2
diff --git a/src/remove.c b/src/remove.c
index d6d6803..87fb32b 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -575,7 +575,7 @@ The following directory is part of the cycle:\n  %s\n"),
              ent->fts_info,
              quote (ent->fts_path),
              PACKAGE_BUGREPORT);
-      assert (0);
+      abort ();
     }
 }

-- 
1.6.3.3.334.g916e1