Re: coreutils 5.92, rm <dir> on Solaris, bad error message

[Jim Meyering]

address@hidden (Bob Proulx) wrote:
> Paul Eggert wrote:
>> Jim Meyering <address@hidden> writes:
>> > +      /* Upon a failed attempt to unlink a directory, Solaris 9 sets
>> > +   errno to EPERM.
>>
>> One suggestion for the comment.  The behavior in question (namely,
>> unlink (dir) fails with errno==EPERM) is not just Solaris 9: it's
>> required by POSIX.  Linux is the odd man out here (though I think that
>> the Linux behavior is better, actually).  See, for example,
>> <http://www.linuxbase.org/spec/refspecs/LSB_1.2.0/gLSB/baselib-unlink-3.html>.
>>
>> So perhaps the comment should read "most non-Linux systems set errno
>> to the POSIX-required value EPERM".
>
> If you don't mind additional word-smithing I would say this
> differently still.  I don't generally like negative logic such as
> non-this or not-that and would prefer positive set groups.
>
>   Traditional and POSIX systems return EPERM but Linux intentionally
>   returns EISDIR to improve upon the situation.

Hi Bob,

Thanks, but I ended up using `non-Linux'.
Since that code is handling the non-Linux case,
I find it clearer to word the comment the same way.

Here's what I installed:

2005-11-02  Jim Meyering  <address@hidden>

        * src/remove.c (remove_entry): Emit a better diagnostic when rm
        (without -r) fails to remove a directory on a non-Linux system.
        This change affects only newer Solaris systems (with priv_*
        functions like priv_allocset).  Reported by Keith Thompson.

        * tests/rm/dir-nonrecur: New file/test for the above fix.
        * tests/rm/Makefile.am (TESTS): Add dir-nonrecur.

Index: src/remove.c
===================================================================
RCS file: /fetish/cu/src/remove.c,v
retrieving revision 1.132
retrieving revision 1.133
diff -u -p -u -r1.132 -r1.133
--- src/remove.c        20 Sep 2005 17:48:02 -0000      1.132
+++ src/remove.c        2 Nov 2005 09:47:39 -0000       1.133
@@ -657,6 +657,19 @@ prompt (Dirstack_state const *ds, char c
   return RM_OK;
 }
 
+/* Return true if FILENAME is a directory (and not a symlink to a directory).
+   Otherwise, including the case in which lstat fails, return false.
+   Do not modify errno.  */
+static inline bool
+is_dir_lstat (char const *filename)
+{
+  struct stat sbuf;
+  int saved_errno = errno;
+  bool is_dir = lstat (filename, &sbuf) == 0 && S_ISDIR (sbuf.st_mode);
+  errno = saved_errno;
+  return is_dir;
+}
+
 #if HAVE_STRUCT_DIRENT_D_TYPE
 
 /* True if the type of the directory entry D is known.  */
@@ -760,6 +773,12 @@ remove_entry (Dirstack_state const *ds, 
 
       DO_UNLINK (filename, x);
 
+      /* Upon a failed attempt to unlink a directory, most non-Linux systems
+        set errno to the POSIX-required value EPERM.  In that case, change
+        errno to EISDIR so that we emit a better diagnostic.  */
+      if (! x->recursive && errno == EPERM && is_dir_lstat (filename))
+       errno = EISDIR;
+
       if (! x->recursive
          || errno == ENOENT || errno == ENOTDIR
          || errno == ELOOP || errno == ENAMETOOLONG)