]> Savannah Git Hosting - gnulib.git/commitdiff
canonicalize: prefer faccessat to stat
authorPaul Eggert <eggert@cs.ucla.edu>
Thu, 24 Dec 2020 19:38:48 +0000 (11:38 -0800)
committerPaul Eggert <eggert@cs.ucla.edu>
Thu, 24 Dec 2020 20:06:48 +0000 (12:06 -0800)
A proper faccessat doesn't have the EOVERFLOW problem, and can be
more efficient as it needn't gather data from the filesystem to
fill in struct stat.  So use stat only if faccessat is absent,
or when checking for symlink loops in canonicalize.c.
* lib/canonicalize-lgpl.c, lib/canonicalize.c:
Include fcntl.h, for AT_EACCESS.
(FACCESSAT_NEVER_EOVERFLOWS): Default to false.
(file_accessible): New function, based on faccessat but with
a fallback to stat and with an EOVERFLOW workaround.
(dir_check): Use it.
(dir_suffix): New static constant.
* lib/canonicalize-lgpl.c (FACCESSAT_NEVER_EOVERFLOWS) [_LIBC]:
Use __ASSUME_FACCESSAT2 to set FACCESSAT_NEVER_EOVERFLOWS
(__faccessat) [!_LIBC]: Define.
(realpath_stk): Use dir_suffix now.
* lib/canonicalize.c (canonicalize_filename_mode_stk):
If logical, don't check each component's existence; just check
at the end, as that's enough.
* m4/canonicalize.m4 (gl_FUNC_CANONICALIZE_FILENAME_MODE)
(gl_CANONICALIZE_LGPL_SEPARATE):
Require gl_FUNC_FACCESSAT_EOVERFLOW,
gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK, and check for faccessat.
(gl_CANONICALIZE_LGPL_SEPARATE): Do not check for readlink,
as the code does not use HAVE_READLINK.
* modules/canonicalize, modules/canonicalize-lgpl (Files):
Add m4/faccessat.m4, m4/lstat.m4.
(Depends-on): Add fcntl-lh.

ChangeLog
lib/canonicalize-lgpl.c
lib/canonicalize.c
m4/canonicalize.m4
modules/canonicalize
modules/canonicalize-lgpl

index 551059e355feb6ab26efec661a996860126ce783..94427d748fb3d60bf441828b64eda112185da135 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,34 @@
 2020-12-24  Paul Eggert  <eggert@cs.ucla.edu>
 
+       canonicalize: prefer faccessat to stat
+       A proper faccessat doesn't have the EOVERFLOW problem, and can be
+       more efficient as it needn't gather data from the filesystem to
+       fill in struct stat.  So use stat only if faccessat is absent,
+       or when checking for symlink loops in canonicalize.c.
+       * lib/canonicalize-lgpl.c, lib/canonicalize.c:
+       Include fcntl.h, for AT_EACCESS.
+       (FACCESSAT_NEVER_EOVERFLOWS): Default to false.
+       (file_accessible): New function, based on faccessat but with
+       a fallback to stat and with an EOVERFLOW workaround.
+       (dir_check): Use it.
+       (dir_suffix): New static constant.
+       * lib/canonicalize-lgpl.c (FACCESSAT_NEVER_EOVERFLOWS) [_LIBC]:
+       Use __ASSUME_FACCESSAT2 to set FACCESSAT_NEVER_EOVERFLOWS
+       (__faccessat) [!_LIBC]: Define.
+       (realpath_stk): Use dir_suffix now.
+       * lib/canonicalize.c (canonicalize_filename_mode_stk):
+       If logical, don't check each component's existence; just check
+       at the end, as that's enough.
+       * m4/canonicalize.m4 (gl_FUNC_CANONICALIZE_FILENAME_MODE)
+       (gl_CANONICALIZE_LGPL_SEPARATE):
+       Require gl_FUNC_FACCESSAT_EOVERFLOW,
+       gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK, and check for faccessat.
+       (gl_CANONICALIZE_LGPL_SEPARATE): Do not check for readlink,
+       as the code does not use HAVE_READLINK.
+       * modules/canonicalize, modules/canonicalize-lgpl (Files):
+       Add m4/faccessat.m4, m4/lstat.m4.
+       (Depends-on): Add fcntl-lh.
+
        faccessat: work around F_OK EOVERFLOW bug
        * doc/posix-functions/faccessat.texi: Mention the problem.
        * lib/faccessat.c (FACCESSAT_NEVER_EOVERFLOWS): Default to 0.
index 431dc5a3bade09bd21254cd15fa80b384e13a34d..3d4218516958625f695ed4aa315ef831b845e939 100644 (file)
@@ -29,6 +29,7 @@
 #include <stdlib.h>
 
 #include <errno.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <stdbool.h>
 #include <stddef.h>
@@ -46,12 +47,19 @@ typedef ptrdiff_t idx_t;
 # define FILE_SYSTEM_PREFIX_LEN(name) 0
 # define IS_ABSOLUTE_FILE_NAME(name) ISSLASH(*(name))
 # define ISSLASH(c) ((c) == '/')
+# include <sysdep.h>
+# ifdef __ASSUME_FACCESSAT2
+#  define FACCESSAT_NEVER_EOVERFLOWS __ASSUME_FACCESSAT2
+# else
+#  define FACCESSAT_NEVER_EOVERFLOWS true
+# endif
 #else
 # define __canonicalize_file_name canonicalize_file_name
 # define __realpath realpath
 # include "idx.h"
 # include "pathmax.h"
 # include "filename.h"
+# define __faccessat faccessat
 # if defined _WIN32 && !defined __CYGWIN__
 #  define __getcwd _getcwd
 # elif HAVE_GETCWD
@@ -88,11 +96,30 @@ typedef ptrdiff_t idx_t;
 #endif
 
 #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT
-# define DOUBLE_SLASH_IS_DISTINCT_ROOT 0
+# define DOUBLE_SLASH_IS_DISTINCT_ROOT false
+#endif
+#ifndef FACCESSAT_NEVER_EOVERFLOWS
+# define FACCESSAT_NEVER_EOVERFLOWS false
 #endif
 
 #if !FUNC_REALPATH_WORKS || defined _LIBC
 
+/* Return true if FILE's existence can be shown, false (setting errno)
+   otherwise.  Follow symbolic links.  */
+static bool
+file_accessible (char const *file)
+{
+# if defined _LIBC || HAVE_FACCESSAT
+  int r = __faccessat (AT_FDCWD, file, F_OK, AT_EACCESS);
+# else
+  struct stat st;
+  int r = __stat (file, &st);
+# endif
+
+  return ((!FACCESSAT_NEVER_EOVERFLOWS && r < 0 && errno == EOVERFLOW)
+          || r == 0);
+}
+
 /* True if concatenating END as a suffix to a file name means that the
    code needs to check that the file name is that of a searchable
    directory, since the canonicalize_filename_mode_stk code won't
@@ -125,18 +152,26 @@ suffix_requires_dir_check (char const *end)
   return false;
 }
 
-/* Return true if DIR is a directory, false (setting errno) otherwise.
+/* Append this to a file name to test whether it is a searchable directory.
+   On POSIX platforms "/" suffices, but "/./" is sometimes needed on
+   macOS 10.13 <https://bugs.gnu.org/30350>, and should also work on
+   platforms like AIX 7.2 that need at least "/.".  */
+
+#if defined _LIBC || defined LSTAT_FOLLOWS_SLASHED_SYMLINK
+static char const dir_suffix[] = "/";
+#else
+static char const dir_suffix[] = "/./";
+#endif
+
+/* Return true if DIR is a searchable dir, false (setting errno) otherwise.
    DIREND points to the NUL byte at the end of the DIR string.
-   Store garbage into DIREND[0] and DIREND[1].  */
+   Store garbage into DIREND[0 .. strlen (dir_suffix)].  */
 
 static bool
 dir_check (char *dir, char *dirend)
 {
-  /* Append "/"; otherwise EOVERFLOW would be ambiguous.  */
-  strcpy (dirend, "/");
-
-  struct stat st;
-  return __stat (dir, &st) == 0 || errno == EOVERFLOW;
+  strcpy (dirend, dir_suffix);
+  return file_accessible (dir);
 }
 
 static idx_t
@@ -280,8 +315,8 @@ realpath_stk (const char *name, char *resolved,
           if (!ISSLASH (dest[-1]))
             *dest++ = '/';
 
-          enum { dir_check_room = sizeof "/" };
-          while (rname + rname_buf->length - dest < startlen + dir_check_room)
+          while (rname + rname_buf->length - dest
+                 < startlen + sizeof dir_suffix)
             {
               idx_t dest_offset = dest - rname;
               if (!scratch_buffer_grow_preserve (rname_buf))
index 62777219ae50c6c74fc1f7e8b4147aa52d15f121..2c88335bf42697a70c227dbf95b336401bc98f94 100644 (file)
@@ -19,6 +19,7 @@
 #include "canonicalize.h"
 
 #include <errno.h>
+#include <fcntl.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdlib.h>
 #include "filename.h"
 
 #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT
-# define DOUBLE_SLASH_IS_DISTINCT_ROOT 0
+# define DOUBLE_SLASH_IS_DISTINCT_ROOT false
+#endif
+#ifndef FACCESSAT_NEVER_EOVERFLOWS
+# define FACCESSAT_NEVER_EOVERFLOWS false
 #endif
 
 #if ISSLASH ('\\')
 # define SLASHES "/"
 #endif
 
+/* Return true if FILE's existence can be shown, false (setting errno)
+   otherwise.  Follow symbolic links.  */
+static bool
+file_accessible (char const *file)
+{
+# if HAVE_FACCESSAT
+  int r = faccessat (AT_FDCWD, file, F_OK, AT_EACCESS);
+# else
+  struct stat st;
+  int r = stat (file, &st);
+# endif
+
+  return ((!FACCESSAT_NEVER_EOVERFLOWS && r < 0 && errno == EOVERFLOW)
+          || r == 0);
+}
+
 /* True if concatenating END as a suffix to a file name means that the
    code needs to check that the file name is that of a searchable
    directory, since the canonicalize_filename_mode_stk code won't
@@ -77,18 +97,26 @@ suffix_requires_dir_check (char const *end)
   return false;
 }
 
-/* Return true if DIR is a directory, false (setting errno) otherwise.
+/* Append this to a file name to test whether it is a searchable directory.
+   On POSIX platforms "/" suffices, but "/./" is sometimes needed on
+   macOS 10.13 <https://bugs.gnu.org/30350>, and should also work on
+   platforms like AIX 7.2 that need at least "/.".  */
+
+#ifdef LSTAT_FOLLOWS_SLASHED_SYMLINK
+static char const dir_suffix[] = "/";
+#else
+static char const dir_suffix[] = "/./";
+#endif
+
+/* Return true if DIR is a searchable dir, false (setting errno) otherwise.
    DIREND points to the NUL byte at the end of the DIR string.
-   Store garbage into DIREND[0] and DIREND[1].  */
+   Store garbage into DIREND[0 .. strlen (dir_suffix)].  */
 
 static bool
 dir_check (char *dir, char *dirend)
 {
-  /* Append "/"; otherwise EOVERFLOW would be ambiguous.  */
-  strcpy (dirend, "/");
-
-  struct stat st;
-  return stat (dir, &st) == 0 || errno == EOVERFLOW;
+  strcpy (dirend, dir_suffix);
+  return file_accessible (dir);
 }
 
 #if !((HAVE_CANONICALIZE_FILE_NAME && FUNC_REALPATH_WORKS)      \
@@ -293,8 +321,8 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
           if (!ISSLASH (dest[-1]))
             *dest++ = '/';
 
-          enum { dir_check_room = sizeof "/" };
-          while (rname + rname_buf->length - dest < startlen + dir_check_room)
+          while (rname + rname_buf->length - dest
+                 < startlen + sizeof dir_suffix)
             {
               idx_t dest_offset = dest - rname;
               if (!scratch_buffer_grow_preserve (rname_buf))
@@ -406,8 +434,9 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
           else if (! (can_exist == CAN_MISSING
                       || (suffix_requires_dir_check (end)
                           ? dir_check (rname, dest)
-                          : ((logical && 0 <= readlink (rname, &discard, 1))
-                             || errno == EINVAL))
+                          : !logical
+                          ? errno == EINVAL
+                          : *end || file_accessible (rname))
                       || (can_exist == CAN_ALL_BUT_LAST
                           && errno == ENOENT
                           && !end[strspn (end, SLASHES)])))
index 14ea3e12fa0d6f99e2f9227152e74476b50f1ee6..c8da4dfcb6b753d239d8d185ddc8d6effb4c1185 100644 (file)
@@ -1,4 +1,4 @@
-# canonicalize.m4 serial 33
+# canonicalize.m4 serial 34
 
 dnl Copyright (C) 2003-2007, 2009-2020 Free Software Foundation, Inc.
 
@@ -11,7 +11,9 @@ dnl with or without modifications, as long as this notice is preserved.
 AC_DEFUN([gl_FUNC_CANONICALIZE_FILENAME_MODE],
 [
   AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
-  AC_CHECK_FUNCS_ONCE([canonicalize_file_name])
+  AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW])
+  AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK])
+  AC_CHECK_FUNCS_ONCE([canonicalize_file_name faccessat])
   AC_REQUIRE([gl_DOUBLE_SLASH_ROOT])
   AC_REQUIRE([gl_FUNC_REALPATH_WORKS])
   if test $ac_cv_func_canonicalize_file_name = no; then
@@ -56,7 +58,9 @@ AC_DEFUN([gl_CANONICALIZE_LGPL],
 AC_DEFUN([gl_CANONICALIZE_LGPL_SEPARATE],
 [
   AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
-  AC_CHECK_FUNCS_ONCE([canonicalize_file_name readlink])
+  AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW])
+  AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK])
+  AC_CHECK_FUNCS_ONCE([canonicalize_file_name faccessat])
 
   dnl On native Windows, we use _getcwd(), regardless whether getcwd() is
   dnl available through the linker option '-loldnames'.
index e1b7607046ad2671c472d39bb25cc2df38a94abe..29919bcdc37b37841bc22933c7dedf12117d03ed 100644 (file)
@@ -5,12 +5,15 @@ Files:
 lib/canonicalize.h
 lib/canonicalize.c
 m4/canonicalize.m4
+m4/faccessat.m4
+m4/lstat.m4
 
 Depends-on:
 attribute
 double-slash-root
 errno
 extensions
+fcntl-h
 file-set
 filename
 free-posix
index e94a898bd692346f8c93e2b347ffe7e56de6512b..dfad9202fb1b2d3f795efcf79517d5506da8fef6 100644 (file)
@@ -5,6 +5,8 @@ Files:
 lib/canonicalize-lgpl.c
 m4/canonicalize.m4
 m4/double-slash-root.m4
+m4/faccessat.m4
+m4/lstat.m4
 
 Depends-on:
 extensions
@@ -12,6 +14,7 @@ stdlib
 nocrash
 double-slash-root [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
 errno             [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+fcntl-h           [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
 filename          [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
 free-posix        [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
 idx               [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]