Re: fchmod-tests, fchmodat tests, lchmod tests: Add more tests

[Bruno Haible]

I wrote:
> It smells like additional tests for fchmod(), fchmodat(), lchmod() could
> uncover bugs on some platforms. So I'm adding more tests.

And indeed, on AIX 7.2 one of these new tests fails:

../../gltests/test-fchmodat.c:60: assertion 'fchmodat (AT_FDCWD, BASE "file/", 
0600, 0) == -1' failed
FAIL test-fchmodat (exit status: 134)

This patch provides a workaround.


2021-01-09  Bruno Haible  <bruno@clisp.org>

        fchmodat: Work around trailing slash bug in fchmodat() on AIX 7.2.
        * m4/fchmodat.m4 (gl_FUNC_FCHMODAT): In the test whether fchmodat works,
        also test for the trailing slashes behaviour. Define
        HAVE_NEARLY_WORKING_FCHMODAT if this is the only missing feature.
        * lib/fchmodat.c (fchmodat): If HAVE_NEARLY_WORKING_FCHMODAT, handle
        trailing slashes here.
        * modules/fchmodat (Depends-on): Sort by condition, not alphabetically.
        * doc/posix-functions/fchmodat.texi: Document the AIX bug.

diff --git a/doc/posix-functions/fchmodat.texi 
b/doc/posix-functions/fchmodat.texi
index 10821b6..aa0c0fd 100644
--- a/doc/posix-functions/fchmodat.texi
+++ b/doc/posix-functions/fchmodat.texi
@@ -14,6 +14,10 @@ glibc 2.3.6, Mac OS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 
3.8, Minix 3.1.8,
 AIX 5.1, HP-UX 11, IRIX 6.5, Solaris 10, Cygwin 1.5.x, mingw, MSVC 14.
 But the replacement function is not safe to be used in libraries and is not 
multithread-safe.
 @item
+This function does not fail when the file name argument ends in a slash
+and (without the slash) names a non-directory, on some platforms:
+AIX 7.2.
+@item
 When given the @code{AT_SYMLINK_NOFOLLOW} flag,
 this function fails with @code{errno} set to @code{ENOTSUP},
 even when the file is not a symbolic link:
diff --git a/lib/fchmodat.c b/lib/fchmodat.c
index d27c0d7..eb6e224 100644
--- a/lib/fchmodat.c
+++ b/lib/fchmodat.c
@@ -38,6 +38,7 @@ orig_fchmodat (int dir, char const *file, mode_t mode, int 
flags)
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
 
 #ifdef __osf__
@@ -63,6 +64,22 @@ orig_fchmodat (int dir, char const *file, mode_t mode, int 
flags)
 int
 fchmodat (int dir, char const *file, mode_t mode, int flags)
 {
+# if HAVE_NEARLY_WORKING_FCHMODAT
+  /* Correct the trailing slash handling.  */
+  size_t len = strlen (file);
+  if (len && file[len - 1] == '/')
+    {
+      struct stat st;
+      if (fstatat (dir, file, &st, flags & AT_SYMLINK_NOFOLLOW) < 0)
+        return -1;
+      if (!S_ISDIR (st.st_mode))
+        {
+          errno = ENOTDIR;
+          return -1;
+        }
+    }
+# endif
+
 # if NEED_FCHMODAT_NONSYMLINK_FIX
   if (flags == AT_SYMLINK_NOFOLLOW)
     {
diff --git a/m4/fchmodat.m4 b/m4/fchmodat.m4
index 0938032..66c0e30 100644
--- a/m4/fchmodat.m4
+++ b/m4/fchmodat.m4
@@ -1,4 +1,4 @@
-# fchmodat.m4 serial 5
+# fchmodat.m4 serial 6
 dnl Copyright (C) 2004-2021 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -16,11 +16,9 @@ AC_DEFUN([gl_FUNC_FCHMODAT],
     HAVE_FCHMODAT=0
   else
     AC_CACHE_CHECK(
-      [whether fchmodat+AT_SYMLINK_NOFOLLOW works on non-symlinks],
+      [whether fchmodat works],
       [gl_cv_func_fchmodat_works],
-      [dnl This test fails on GNU/Linux with glibc 2.31 (but not on
-       dnl GNU/kFreeBSD nor GNU/Hurd) and Cygwin 2.9.
-       AC_RUN_IFELSE(
+      [AC_RUN_IFELSE(
          [AC_LANG_PROGRAM(
             [
               AC_INCLUDES_DEFAULT[
@@ -44,27 +42,49 @@ AC_DEFUN([gl_FUNC_FCHMODAT],
             [[
               int permissive = S_IRWXU | S_IRWXG | S_IRWXO;
               int desired = S_IRUSR | S_IWUSR;
-              static char const f[] = "conftest.fchmodat";
+              int result = 0;
+              #define file "conftest.fchmodat"
               struct stat st;
-              if (creat (f, permissive) < 0)
+              if (creat (file, permissive) < 0)
                 return 1;
-              if (fchmodat (AT_FDCWD, f, desired, AT_SYMLINK_NOFOLLOW) != 0)
+              /* Test whether fchmodat rejects a trailing slash on a 
non-directory.
+                 This test fails on AIX 7.2.  */
+              if (fchmodat (AT_FDCWD, file "/", desired, 0) == 0)
+                result |= 2;
+              /* Test whether fchmodat+AT_SYMLINK_NOFOLLOW works on 
non-symlinks.
+                 This test fails on GNU/Linux with glibc 2.31 (but not on
+                 GNU/kFreeBSD nor GNU/Hurd) and Cygwin 2.9.  */
+              if (fchmodat (AT_FDCWD, file, desired, AT_SYMLINK_NOFOLLOW) != 0)
+                result |= 4;
+              if (stat (file, &st) != 0)
                 return 1;
-              if (stat (f, &st) != 0)
-                return 1;
-              return ! ((st.st_mode & permissive) == desired);
+              if ((st.st_mode & permissive) != desired)
+                result |= 4;
+              return result;
             ]])],
          [gl_cv_func_fchmodat_works=yes],
-         [gl_cv_func_fchmodat_works=no],
+         [case $? in
+            2) gl_cv_func_fchmodat_works='nearly' ;;
+            *) gl_cv_func_fchmodat_works=no ;;
+          esac
+         ],
          [case "$host_os" in
-            dnl Guess no on Linux with glibc and Cygwin, yes otherwise.
+                                  # Guess no on Linux with glibc and Cygwin.
             linux-gnu* | cygwin*) gl_cv_func_fchmodat_works="guessing no" ;;
+                                  # Guess 'nearly' on AIX.
+            aix*)                 gl_cv_func_fchmodat_works="guessing nearly" 
;;
+                                  # If we don't know, obey 
--enable-cross-guesses.
             *)                    
gl_cv_func_fchmodat_works="$gl_cross_guess_normal" ;;
           esac
          ])
        rm -f conftest.fchmodat])
-    case $gl_cv_func_fchmodat_works in
+    case "$gl_cv_func_fchmodat_works" in
       *yes) ;;
+      *nearly)
+        AC_DEFINE([HAVE_NEARLY_WORKING_FCHMODAT], [1],
+          [Define to 1 if fchmodat works, except for the trailing slash 
handling.])
+        REPLACE_FCHMODAT=1
+        ;;
       *)
         AC_DEFINE([NEED_FCHMODAT_NONSYMLINK_FIX], [1],
           [Define to 1 if fchmodat+AT_SYMLINK_NOFOLLOW does not work right on 
non-symlinks.])
diff --git a/modules/fchmodat b/modules/fchmodat
index 975889b..17cfe39 100644
--- a/modules/fchmodat
+++ b/modules/fchmodat
@@ -9,20 +9,20 @@ m4/fchmodat.m4
 Depends-on:
 sys_stat
 extensions
-at-internal     [test $HAVE_FCHMODAT = 0]
-c99             [test $REPLACE_FCHMODAT = 1]
 errno           [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
+fcntl-h         [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
+unistd          [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
+intprops        [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
+c99             [test $REPLACE_FCHMODAT = 1]
+fstatat         [test $REPLACE_FCHMODAT = 1]
+at-internal     [test $HAVE_FCHMODAT = 0]
 extern-inline   [test $HAVE_FCHMODAT = 0]
 fchdir          [test $HAVE_FCHMODAT = 0]
-fcntl-h         [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
 filename        [test $HAVE_FCHMODAT = 0]
-fstatat         [test $REPLACE_FCHMODAT = 1]
-intprops        [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
 lchmod          [test $HAVE_FCHMODAT = 0]
 openat-die      [test $HAVE_FCHMODAT = 0]
 openat-h        [test $HAVE_FCHMODAT = 0]
 save-cwd        [test $HAVE_FCHMODAT = 0]
-unistd          [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
 
 configure.ac:
 gl_FUNC_FCHMODAT