+2020-02-22 Paul Eggert <eggert@cs.ucla.edu>
+
+ fchmodat, lchmod: simplify
+ It appears that we may have overengineered lchmod and fchmodat,
+ in that the code was prepared for some hypothetical platforms but
+ was so complicated that it was hard to understand. I attempted to
+ improve the situation by simplifying the code when this
+ simplification should not hurt on real platforms; we can re-add
+ complexity later to port to platforms I didn’t know about.
+ * lib/fchmodat.c (fchmodat):
+ * lib/lchmod.c (lchmod):
+ Put the ‘defined __linux__ || defined __ANDROID__’ #ifdef only
+ around the /proc code that needs it.
+ * lib/fchmodat.c (fchmodat): Coalese calls to orig_fchmodat.
+ * lib/lchmod.c (__need_system_sys_stat_h): Omit; no longer needed.
+ Do not include <config.h> twice.
+ (orig_lchmod) [HAVE_LCHMOD]: Remove, since we need not wrap
+ lchmod on any known hosts.
+ (lchmod): Do not defer to fchmodat, so that the lchmod module
+ need not depend on the fchmodat module (which is a circular
+ dependency). Do not use openat, since ‘open’ suffices.
+ Coalesce calls to lchmod/chmod.
+ * lib/lchmod.c, lib/sys_stat.in.h (lchmod):
+ * m4/sys_stat_h.m4 (REPLACE_FSTAT):
+ * modules/lchmod (Depends-on, configure.ac):
+ * modules/sys_stat (Depends-on):
+ Do not worry about replacing lchmod, since that shouldn’t happen.
+ * m4/lchmod.m4 (gl_FUNC_LCHMOD): Do not check for fchmodat.
+ Do not worry about whether lchmod works on non-symlinks,
+ since every known lchmod works on non-symlinks.
+ * modules/lchmod (Depends-on):
+ Remove circular dependency on fchmodat.
+
2020-02-22 Bruno Haible <bruno@clisp.org>
lchmod: Fix link error on Solaris 10 (regression from 2020-02-16).
{
struct stat st;
-# if defined O_PATH && defined AT_EMPTY_PATH \
- && (defined __linux__ || defined __ANDROID__)
+# if defined O_PATH && defined AT_EMPTY_PATH
/* Open a file descriptor with O_NOFOLLOW, to make sure we don't
follow symbolic links, if /proc is mounted. O_PATH is used to
avoid a failure if the file is not readable.
/* Up to Linux 5.3 at least, when FILE refers to a symbolic link, the
chmod call below will change the permissions of the symbolic link
- - which is undersired - and on many file systems (ext4, btrfs, jfs,
+ - which is undesired - and on many file systems (ext4, btrfs, jfs,
xfs, ..., but not reiserfs) fail with error EOPNOTSUPP - which is
misleading. Therefore test for a symbolic link explicitly.
Use fstatat because fstat does not work on O_PATH descriptors
return -1;
}
+# if defined __linux__ || defined __ANDROID__
static char const fmt[] = "/proc/self/fd/%d";
char buf[sizeof fmt - sizeof "%d" + INT_BUFSIZE_BOUND (int)];
sprintf (buf, fmt, fd);
errno = chmod_errno;
return chmod_result;
}
- /* /proc is not mounted. */
- /* Fall back on orig_fchmodat, despite the race. */
- return orig_fchmodat (dir, file, mode, 0);
-# elif (defined __linux__ || defined __ANDROID__) || !HAVE_LCHMOD
+# endif
+ /* /proc is not mounted or would not work as in GNU/Linux. */
+
+# else
int fstatat_result = fstatat (dir, file, &st, AT_SYMLINK_NOFOLLOW);
if (fstatat_result != 0)
return fstatat_result;
errno = EOPNOTSUPP;
return -1;
}
- /* Fall back on orig_fchmodat, despite the race. */
- return orig_fchmodat (dir, file, mode, 0);
-# else
- return orig_fchmodat (dir, file, mode, 0);
# endif
+
+ /* Fall back on orig_fchmodat with no flags, despite a possible race. */
+ flags = 0;
}
# endif
#include <config.h>
-/* If the user's config.h happens to include <sys/stat.h>, let it include only
- the system's <sys/stat.h> here, so that orig_fchmodat doesn't recurse to
- rpl_fchmodat. */
-#define __need_system_sys_stat_h
-#include <config.h>
-
/* Specification. */
#include <sys/stat.h>
-#undef __need_system_sys_stat_h
-
-#if HAVE_LCHMOD
-static inline int
-orig_lchmod (char const *file, mode_t mode)
-{
- return lchmod (file, mode);
-}
-#endif
#include <errno.h>
#include <fcntl.h>
int
lchmod (char const *file, mode_t mode)
{
-#if HAVE_FCHMODAT
- /* Gnulib's fchmodat contains the workaround. No need to duplicate it
- here. */
- return fchmodat (AT_FDCWD, file, mode, AT_SYMLINK_NOFOLLOW);
-#elif NEED_LCHMOD_NONSYMLINK_FIX \
- && defined AT_FDCWD && defined O_PATH && defined AT_EMPTY_PATH \
- && (defined __linux__ || defined __ANDROID__) /* newer Linux */
+#if defined O_PATH && defined AT_EMPTY_PATH
/* Open a file descriptor with O_NOFOLLOW, to make sure we don't
follow symbolic links, if /proc is mounted. O_PATH is used to
avoid a failure if the file is not readable.
Cf. <https://sourceware.org/bugzilla/show_bug.cgi?id=14578> */
- int fd = openat (AT_FDCWD, file, O_PATH | O_NOFOLLOW | O_CLOEXEC);
+ int fd = open (file, O_PATH | O_NOFOLLOW | O_CLOEXEC);
if (fd < 0)
return fd;
/* Up to Linux 5.3 at least, when FILE refers to a symbolic link, the
chmod call below will change the permissions of the symbolic link
- - which is undersired - and on many file systems (ext4, btrfs, jfs,
+ - which is undesired - and on many file systems (ext4, btrfs, jfs,
xfs, ..., but not reiserfs) fail with error EOPNOTSUPP - which is
misleading. Therefore test for a symbolic link explicitly.
Use fstatat because fstat does not work on O_PATH descriptors
return -1;
}
+# if defined __linux__ || defined __ANDROID__
static char const fmt[] = "/proc/self/fd/%d";
char buf[sizeof fmt - sizeof "%d" + INT_BUFSIZE_BOUND (int)];
sprintf (buf, fmt, fd);
errno = chmod_errno;
return chmod_result;
}
- /* /proc is not mounted. */
- /* Fall back on chmod, despite the race. */
- return chmod (file, mode);
+# endif
+ /* /proc is not mounted or would not work as in GNU/Linux. */
+
#elif HAVE_LSTAT
-# if (NEED_LCHMOD_NONSYMLINK_FIX && (defined __linux__ || defined __ANDROID__)) \
- || !HAVE_LCHMOD /* older Linux, Solaris 10 */
struct stat st;
int lstat_result = lstat (file, &st);
if (lstat_result != 0)
errno = EOPNOTSUPP;
return -1;
}
- /* Fall back on chmod, despite the race. */
- return chmod (file, mode);
-# else /* GNU/kFreeBSD, GNU/Hurd, macOS, FreeBSD, NetBSD, HP-UX */
- return orig_lchmod (file, mode);
-# endif
-#else /* native Windows */
- return chmod (file, mode);
#endif
+
+ /* Fall back on chmod, despite a possible race. */
+ return chmod (file, mode);
}
#if @GNULIB_LCHMOD@
/* Change the mode of FILENAME to MODE, without dereferencing it if FILENAME
denotes a symbolic link. */
-# if @REPLACE_LCHMOD@
-# if !(defined __cplusplus && defined GNULIB_NAMESPACE)
-# undef lchmod
-# define lchmod rpl_lchmod
-# endif
-_GL_FUNCDECL_RPL (lchmod, int,
- (char const *filename, mode_t mode)
- _GL_ARG_NONNULL ((1)));
-_GL_CXXALIAS_RPL (lchmod, int,
- (char const *filename, mode_t mode));
-# else
-# if !@HAVE_LCHMOD@ || defined __hpux
+# if !@HAVE_LCHMOD@ || defined __hpux
_GL_FUNCDECL_SYS (lchmod, int, (const char *filename, mode_t mode)
_GL_ARG_NONNULL ((1)));
-# endif
-_GL_CXXALIAS_SYS (lchmod, int, (const char *filename, mode_t mode));
# endif
+_GL_CXXALIAS_SYS (lchmod, int, (const char *filename, mode_t mode));
_GL_CXXALIASWARN (lchmod);
#elif defined GNULIB_POSIXCHECK
# undef lchmod
-#serial 6
+#serial 7
dnl Copyright (C) 2005-2006, 2008-2020 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
- AC_CHECK_FUNCS_ONCE([fchmodat lchmod lstat])
+ AC_CHECK_FUNCS_ONCE([lchmod lstat])
if test "$ac_cv_func_lchmod" = no; then
HAVE_LCHMOD=0
- else
- AC_CACHE_CHECK([whether lchmod works on non-symlinks],
- [gl_cv_func_lchmod_works],
- [AC_RUN_IFELSE(
- [AC_LANG_PROGRAM(
- [
- AC_INCLUDES_DEFAULT[
- #ifndef S_IRUSR
- #define S_IRUSR 0400
- #endif
- #ifndef S_IWUSR
- #define S_IWUSR 0200
- #endif
- #ifndef S_IRWXU
- #define S_IRWXU 0700
- #endif
- #ifndef S_IRWXG
- #define S_IRWXG 0070
- #endif
- #ifndef S_IRWXO
- #define S_IRWXO 0007
- #endif
- ]],
- [[
- int permissive = S_IRWXU | S_IRWXG | S_IRWXO;
- int desired = S_IRUSR | S_IWUSR;
- static char const f[] = "conftest.lchmod";
- struct stat st;
- if (creat (f, permissive) < 0)
- return 1;
- if (lchmod (f, desired) != 0)
- return 1;
- if (stat (f, &st) != 0)
- return 1;
- return ! ((st.st_mode & permissive) == desired);
- ]])],
- [gl_cv_func_lchmod_works=yes],
- [gl_cv_func_lchmod_works=no],
- [case "$host_os" in
- dnl Guess no on Linux with glibc, yes otherwise.
- linux-gnu*) gl_cv_func_lchmod_works="guessing no" ;;
- *) gl_cv_func_lchmod_works="$gl_cross_guess_normal" ;;
- esac
- ])
- rm -f conftest.lchmod])
- case $gl_cv_func_lchmod_works in
- *yes) ;;
- *)
- AC_DEFINE([NEED_LCHMOD_NONSYMLINK_FIX], [1],
- [Define to 1 if lchmod does not work right on non-symlinks.])
- REPLACE_LCHMOD=1
- ;;
- esac
fi
])
REPLACE_FSTAT=0; AC_SUBST([REPLACE_FSTAT])
REPLACE_FSTATAT=0; AC_SUBST([REPLACE_FSTATAT])
REPLACE_FUTIMENS=0; AC_SUBST([REPLACE_FUTIMENS])
- REPLACE_LCHMOD=0; AC_SUBST([REPLACE_LCHMOD])
REPLACE_LSTAT=0; AC_SUBST([REPLACE_LSTAT])
REPLACE_MKDIR=0; AC_SUBST([REPLACE_MKDIR])
REPLACE_MKFIFO=0; AC_SUBST([REPLACE_MKFIFO])
m4/lchmod.m4
Depends-on:
-errno [test $HAVE_LCHMOD = 0 || test $REPLACE_LCHMOD = 1]
+errno [test $HAVE_LCHMOD = 0]
extensions
-fcntl-h [test $HAVE_LCHMOD = 0 || test $REPLACE_LCHMOD = 1]
-fchmodat [test $HAVE_LCHMOD = 0 || test $REPLACE_LCHMOD = 1]
-intprops [test $HAVE_LCHMOD = 0 || test $REPLACE_LCHMOD = 1]
-lstat [test $HAVE_LCHMOD = 0 || test $REPLACE_LCHMOD = 1]
+fcntl-h [test $HAVE_LCHMOD = 0]
+intprops [test $HAVE_LCHMOD = 0]
+lstat [test $HAVE_LCHMOD = 0]
sys_stat
-unistd [test $HAVE_LCHMOD = 0 || test $REPLACE_LCHMOD = 1]
+unistd [test $HAVE_LCHMOD = 0]
configure.ac:
gl_FUNC_LCHMOD
-if test $HAVE_LCHMOD = 0 || test $REPLACE_LCHMOD = 1; then
+if test $HAVE_LCHMOD = 0; then
AC_LIBOBJ([lchmod])
gl_PREREQ_LCHMOD
fi
-e 's|@''REPLACE_FSTAT''@|$(REPLACE_FSTAT)|g' \
-e 's|@''REPLACE_FSTATAT''@|$(REPLACE_FSTATAT)|g' \
-e 's|@''REPLACE_FUTIMENS''@|$(REPLACE_FUTIMENS)|g' \
- -e 's|@''REPLACE_LCHMOD''@|$(REPLACE_LCHMOD)|g' \
-e 's|@''REPLACE_LSTAT''@|$(REPLACE_LSTAT)|g' \
-e 's|@''REPLACE_MKDIR''@|$(REPLACE_MKDIR)|g' \
-e 's|@''REPLACE_MKFIFO''@|$(REPLACE_MKFIFO)|g' \