Re: coreutils-8.13.29-43a9

[Jim Meyering]

Jim Meyering wrote:
> Bruno Haible wrote:
>> On HP-UX 11.31 with cc:
>> FAIL: rm/deep-2 (exit: 1)
>> =========================
>> + : perl
>> + perl -e 'my $d = "x" x 200; foreach my $i (1..52)' -e ' { mkdir ($d,
>> 0700) && chdir $d or die "$!" }'
>> + cd ..
>> + echo n
>> + rm ---presume-input-tty -r x
>> rm: cannot remove x/xxxxxxxxx... ...':
>> File name too long
>> + fail=1
>
> That name is 1207 bytes long, which must be larger than HPUX 11.31's PATH_MAX.
>
> remove.c's write_protected_non_symlink must be
> calling euidaccess_stat with the long "full_name".
> Obviously, that would fail with "File name too long".
>
> This is a problem on HPUX because it lacks *at-function support.
> One way to work around it would be to change this:
>
> -   if (!openat_needs_fchdir ())
> +   if (1)
>
> But that would make rm use the fully emulated faccessat, which may actually
> call fchdir, and which fails in the unusual event that save_cwd fails.
>
> This is all in a very deep dark corner, so my first reaction reluctance
> to compromise the implementation just to accommodate systems that lack
> openat and/or /proc/self/fd support.  However, once my brain engaged,
> I realized that using "imperfect" at-function emulation here would have
> no impact.  What happens when we determine this file is removable?
>
> We unlink it via unlinkat.
>
> That unlinkat function uses the very same underlying emulation code
> that faccessat does, so there is no reason to limit faccessat
> use to when we have adequate openat/proc support.  Just use it all
> of the time and remove the ugly hacks.

While the hacks were ugly (and the code is now gone), I've deliberately left
most of the comments as a warning to anyone who thinks openat-style functions
are *not* an essential part of any system, these days.

> Patch coming up...

Here's the patch:
It's not often I get to remove 3 files from coreutils, these days.

>From a6ed806f738c06a380e418df9ef254949118dfa5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 9 Oct 2011 10:52:52 +0200
Subject: [PATCH] rm: do not resort to stat'ing very long names even on
 deficient systems

This change affects only uses of rm with -r but without -f and on
systems lacking support for openat-like functions.
* src/remove.c (write_protected_non_symlink): Call faccessat
unconditionally.  Thus we no longer need euidaccess_stat, which was
the sole function used here to operate on a full relative file name.
Remove full_name parameter and update caller.
Other than the removal of the openat_needs_fchdir call, this change
affects only systems that have neither *at function support nor the
/proc/self/fd support required to emulate those *at functions.
* lib/euidaccess-stat.h: Remove file.
* lib/euidaccess-stat.c: Likewise.
* m4/euidaccess-stat.m4: Likewise.
* m4/prereq.m4 (gl_PREREQ): Don't require gl_EUIDACCESS_STAT.
Prompted by a report from Bruno Haible that the rm/deep-2
test was failing on HP-UX 11.31.
See http://thread.gmane.org/gmane.comp.gnu.coreutils.general/1748
---
 lib/euidaccess-stat.c |  134 -------------------------------------------------
 lib/euidaccess-stat.h |    5 --
 m4/euidaccess-stat.m4 |   11 ----
 m4/prereq.m4          |    3 +-
 src/remove.c          |   35 ++-----------
 5 files changed, 5 insertions(+), 183 deletions(-)
 delete mode 100644 lib/euidaccess-stat.c
 delete mode 100644 lib/euidaccess-stat.h
 delete mode 100644 m4/euidaccess-stat.m4

diff --git a/lib/euidaccess-stat.c b/lib/euidaccess-stat.c
deleted file mode 100644
index 46c04b8..0000000
--- a/lib/euidaccess-stat.c
+++ /dev/null
@@ -1,134 +0,0 @@
-/* euidaccess-stat -- check if effective user id can access lstat'd file
-   This function is probably useful only for choosing whether to issue
-   a prompt in an implementation of POSIX-specified rm.
-
-   Copyright (C) 2005-2006, 2009-2011 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
-   the Free Software Foundation, either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-/* Adapted for use in GNU remove.c by Jim Meyering.  */
-
-#include <config.h>
-
-#include "euidaccess-stat.h"
-
-#include <unistd.h>
-
-#include "stat-macros.h"
-
-/* Return true if the current user has permission of type MODE
-   on the file from which stat buffer *ST was obtained, ignoring
-   ACLs, attributes, `read-only'ness, etc...
-   Otherwise, return false.
-
-   Like the reentrant version of euidaccess, but starting with
-   a stat buffer rather than a file name.  Hence, this function
-   never calls access or accessx, and doesn't take into account
-   whether the file has ACLs or other attributes, or resides on
-   a read-only file system.  */
-
-bool
-euidaccess_stat (struct stat const *st, int mode)
-{
-  uid_t euid;
-  unsigned int granted;
-
-  /* Convert the mode to traditional form, clearing any bogus bits.  */
-  if (R_OK == 4 && W_OK == 2 && X_OK == 1 && F_OK == 0)
-    mode &= 7;
-  else
-    mode = ((mode & R_OK ? 4 : 0)
-            + (mode & W_OK ? 2 : 0)
-            + (mode & X_OK ? 1 : 0));
-
-  if (mode == 0)
-    return true;               /* The file exists.  */
-
-  euid = geteuid ();
-
-  /* The super-user can read and write any file, and execute any file
-     that anyone can execute.  */
-  if (euid == 0 && ((mode & X_OK) == 0
-                    || (st->st_mode & (S_IXUSR | S_IXGRP | S_IXOTH))))
-    return true;
-
-  /* Convert the file's permission bits to traditional form.  */
-  if (   S_IRUSR == (4 << 6)
-      && S_IWUSR == (2 << 6)
-      && S_IXUSR == (1 << 6)
-      && S_IRGRP == (4 << 3)
-      && S_IWGRP == (2 << 3)
-      && S_IXGRP == (1 << 3)
-      && S_IROTH == (4 << 0)
-      && S_IWOTH == (2 << 0)
-      && S_IXOTH == (1 << 0))
-    granted = st->st_mode;
-  else
-    granted = (  (st->st_mode & S_IRUSR ? 4 << 6 : 0)
-               + (st->st_mode & S_IWUSR ? 2 << 6 : 0)
-               + (st->st_mode & S_IXUSR ? 1 << 6 : 0)
-               + (st->st_mode & S_IRGRP ? 4 << 3 : 0)
-               + (st->st_mode & S_IWGRP ? 2 << 3 : 0)
-               + (st->st_mode & S_IXGRP ? 1 << 3 : 0)
-               + (st->st_mode & S_IROTH ? 4 << 0 : 0)
-               + (st->st_mode & S_IWOTH ? 2 << 0 : 0)
-               + (st->st_mode & S_IXOTH ? 1 << 0 : 0));
-
-  if (euid == st->st_uid)
-    granted >>= 6;
-  else
-    {
-      gid_t egid = getegid ();
-      if (egid == st->st_gid || group_member (st->st_gid))
-        granted >>= 3;
-    }
-
-  if ((mode & ~granted) == 0)
-    return true;
-
-  return false;
-}
-
-
-#ifdef TEST
-# include <errno.h>
-# include <stdio.h>
-# include <stdlib.h>
-
-# include "error.h"
-# define _(msg) msg
-
-char *program_name;
-
-int
-main (int argc, char **argv)
-{
-  char *file;
-  int mode;
-  bool ok;
-  struct stat st;
-
-  program_name = argv[0];
-  if (argc < 3)
-    abort ();
-  file = argv[1];
-  mode = atoi (argv[2]);
-  if (lstat (file, &st) != 0)
-    error (EXIT_FAILURE, errno, _("cannot stat %s"), file);
-
-  ok = euidaccess_stat (&st, mode);
-  printf ("%s: %s\n", file, ok ? "y" : "n");
-  return 0;
-}
-#endif
diff --git a/lib/euidaccess-stat.h b/lib/euidaccess-stat.h
deleted file mode 100644
index de24961..0000000
--- a/lib/euidaccess-stat.h
+++ /dev/null
@@ -1,5 +0,0 @@
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <stdbool.h>
-
-bool euidaccess_stat (struct stat const *st, int mode);
diff --git a/m4/euidaccess-stat.m4 b/m4/euidaccess-stat.m4
deleted file mode 100644
index 1f97172..0000000
--- a/m4/euidaccess-stat.m4
+++ /dev/null
@@ -1,11 +0,0 @@
-# serial 1
-dnl Copyright (C) 2005, 2009-2011 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.
-
-AC_DEFUN([gl_EUIDACCESS_STAT],
-[
-  AC_LIBSOURCES([euidaccess-stat.c, euidaccess-stat.h])
-  AC_LIBOBJ([euidaccess-stat])
-])
diff --git a/m4/prereq.m4 b/m4/prereq.m4
index c3feecb..574ac82 100644
--- a/m4/prereq.m4
+++ b/m4/prereq.m4
@@ -1,4 +1,4 @@
-#serial 77
+#serial 78

 dnl We use gl_ for non Autoconf macros.
 m4_pattern_forbid([^gl_[ABCDEFGHIJKLMNOPQRSTUVXYZ]])dnl
@@ -36,7 +36,6 @@ AC_DEFUN([gl_PREREQ],
   # Invoke macros of modules that may migrate into gnulib.
   # There's no need to list gnulib modules here, since gnulib-tool
   # handles that; see ../bootstrap.conf.
-  AC_REQUIRE([gl_EUIDACCESS_STAT])
   AC_REQUIRE([gl_FD_REOPEN])
   AC_REQUIRE([gl_FUNC_XATTR])
   AC_REQUIRE([gl_FUNC_XFTS])
diff --git a/src/remove.c b/src/remove.c
index 3814232..c650543 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -23,7 +23,6 @@

 #include "system.h"
 #include "error.h"
-#include "euidaccess-stat.h"
 #include "file-type.h"
 #include "ignore-value.h"
 #include "quote.h"
@@ -106,13 +105,10 @@ cache_stat_ok (struct stat *st)
 /* Return 1 if FILE is an unwritable non-symlink,
    0 if it is writable or some other type of file,
    -1 and set errno if there is some problem in determining the answer.
-   Use FULL_NAME only if necessary.
-   Set *BUF to the file status.
-   This is to avoid calling euidaccess when FILE is a symlink.  */
+   Set *BUF to the file status.  */
 static int
 write_protected_non_symlink (int fd_cwd,
                              char const *file,
-                             char const *full_name,
                              struct stat *buf)
 {
   if (can_write_any_file ())
@@ -170,32 +166,10 @@ write_protected_non_symlink (int fd_cwd,
         mess up with long file names). */

   {
-    /* This implements #1: on decent systems, either faccessat is
-       native or /proc/self/fd allows us to skip a chdir.  */
-    if (!openat_needs_fchdir ())
-      {
-        if (faccessat (fd_cwd, file, W_OK, AT_EACCESS) == 0)
-          return 0;
-
-        return errno == EACCES ? 1 : -1;
-      }
-
-    /* This implements #5: */
-    size_t file_name_len = strlen (full_name);
-
-    if (MIN (PATH_MAX, 8192) <= file_name_len)
-      return ! euidaccess_stat (buf, W_OK);
-    if (euidaccess (full_name, W_OK) == 0)
+    if (faccessat (fd_cwd, file, W_OK, AT_EACCESS) == 0)
       return 0;
-    if (errno == EACCES)
-      {
-        errno = 0;
-        return 1;
-      }

-    /* Perhaps some other process has removed the file, or perhaps this
-       is a buggy NFS client.  */
-    return -1;
+    return errno == EACCES ? 1 : -1;
   }
 }

@@ -244,8 +218,7 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
       && ((x->interactive == RMI_ALWAYS) || x->stdin_tty)
       && dirent_type != DT_LNK)
     {
-      write_protected = write_protected_non_symlink (fd_cwd, filename,
-                                                     full_name, sbuf);
+      write_protected = write_protected_non_symlink (fd_cwd, filename, sbuf);
       wp_errno = errno;
     }

--
1.7.7.rc0.362.g5a14