]> Savannah Git Hosting - gnulib.git/commitdiff
careadlinkat: speedup for GCC 10 with GCC_LINT
authorPaul Eggert <eggert@cs.ucla.edu>
Mon, 17 Aug 2020 19:39:48 +0000 (12:39 -0700)
committerPaul Eggert <eggert@cs.ucla.edu>
Mon, 17 Aug 2020 19:41:42 +0000 (12:41 -0700)
Inspired by a suggestion by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-08/msg00155.html
* lib/careadlinkat.c (STACK_BUF_SIZE): New constant.
(readlink_stk): New function, with most of the old careadlinkat
contents and with a new STACK_BUF arg.  Inline it in GCC 10
if GCC_LINT.
(careadlinkat): Use the new function for everything but the
stack buffer.

ChangeLog
lib/careadlinkat.c

index a5cd6e3167718aea22b44b54d3710dbe90e6ce1b..cd00997a920b6aad6c6db2da65ac2a0f2d79dfa0 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2020-08-17  Paul Eggert  <eggert@cs.ucla.edu>
 
+       careadlinkat: speedup for GCC 10 with GCC_LINT
+       Inspired by a suggestion by Bruno Haible in:
+       https://lists.gnu.org/r/bug-gnulib/2020-08/msg00155.html
+       * lib/careadlinkat.c (STACK_BUF_SIZE): New constant.
+       (readlink_stk): New function, with most of the old careadlinkat
+       contents and with a new STACK_BUF arg.  Inline it in GCC 10
+       if GCC_LINT.
+       (careadlinkat): Use the new function for everything but the
+       stack buffer.
+
        * build-aux/gcc-warning.spec: Update comments.
 
 2020-08-17  Bruno Haible  <bruno@clisp.org>
index 1aa04363daccdf98b7d7c47c90f17e5b674474f6..e43aa42d5c4dd102979811fd04a21a850193858c 100644 (file)
 
 #include "allocator.h"
 
-/* Assuming the current directory is FD, get the symbolic link value
-   of FILENAME as a null-terminated string and put it into a buffer.
-   If FD is AT_FDCWD, FILENAME is interpreted relative to the current
-   working directory, as in openat.
-
-   If the link is small enough to fit into BUFFER put it there.
-   BUFFER's size is BUFFER_SIZE, and BUFFER can be null
-   if BUFFER_SIZE is zero.
-
-   If the link is not small, put it into a dynamically allocated
-   buffer managed by ALLOC.  It is the caller's responsibility to free
-   the returned value if it is nonnull and is not BUFFER.  A null
-   ALLOC stands for the standard allocator.
-
-   The PREADLINKAT function specifies how to read links.  It operates
-   like POSIX readlinkat()
-   <https://pubs.opengroup.org/onlinepubs/9699919799/functions/readlink.html>
-   but can assume that its first argument is the same as FD.
-
-   If successful, return the buffer address; otherwise return NULL and
-   set errno.  */
-
-char *
-careadlinkat (int fd, char const *filename,
+enum { STACK_BUF_SIZE = 1024 };
+
+/* Act like careadlinkat (see below), with an additional argument
+   STACK_BUF that can be used as temporary storage.
+
+   If GCC_LINT is defined, do not inline this function with GCC 10.1
+   and later, to avoid creating a pointer to the stack that GCC
+   -Wreturn-local-addr incorrectly complains about.  See:
+   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644
+   Although the noinline attribute can hurt performance a bit, no better way
+   to pacify GCC is known; even an explicit #pragma does not pacify GCC.
+   When the GCC bug is fixed this workaround should be limited to the
+   broken GCC versions.  */
+#if (defined GCC_LINT || defined lint) && _GL_GNUC_PREREQ (10, 1)
+__attribute__ ((__noinline__))
+#endif
+static char *
+readlink_stk (int fd, char const *filename,
               char *buffer, size_t buffer_size,
               struct allocator const *alloc,
-              ssize_t (*preadlinkat) (int, char const *, char *, size_t))
+              ssize_t (*preadlinkat) (int, char const *, char *, size_t),
+              char stack_buf[STACK_BUF_SIZE])
 {
   char *buf;
   size_t buf_size;
   size_t buf_size_max =
     SSIZE_MAX < SIZE_MAX ? (size_t) SSIZE_MAX + 1 : SIZE_MAX;
-  char stack_buf[1024];
-
-#if (defined GCC_LINT || defined lint) && _GL_GNUC_PREREQ (10, 1)
-  /* Pacify preadlinkat without creating a pointer to the stack
-     that a broken gcc -Wreturn-local-addr would cry wolf about.  See:
-     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95044
-     This workaround differs from the mainline code, but
-     no other way to pacify GCC 10.1.0 is known; even an explicit
-     #pragma does not pacify GCC.  When the GCC bug is fixed this
-     workaround should be limited to the broken GCC versions.  */
-# define WORK_AROUND_GCC_BUG_95044
-#endif
 
   if (! alloc)
     alloc = &stdlib_allocator;
 
   if (!buffer)
     {
-#ifdef WORK_AROUND_GCC_BUG_95044
-      buffer = alloc->allocate (sizeof stack_buf);
-#else
-      /* Allocate the initial buffer on the stack.  This way, in the
-         common case of a symlink of small size, we get away with a
-         single small malloc() instead of a big malloc() followed by a
-         shrinking realloc().  */
       buffer = stack_buf;
-#endif
-      buffer_size = sizeof stack_buf;
+      buffer_size = STACK_BUF_SIZE;
     }
 
   buf = buffer;
@@ -172,3 +147,44 @@ careadlinkat (int fd, char const *filename,
   errno = ENOMEM;
   return NULL;
 }
+
+
+/* Assuming the current directory is FD, get the symbolic link value
+   of FILENAME as a null-terminated string and put it into a buffer.
+   If FD is AT_FDCWD, FILENAME is interpreted relative to the current
+   working directory, as in openat.
+
+   If the link is small enough to fit into BUFFER put it there.
+   BUFFER's size is BUFFER_SIZE, and BUFFER can be null
+   if BUFFER_SIZE is zero.
+
+   If the link is not small, put it into a dynamically allocated
+   buffer managed by ALLOC.  It is the caller's responsibility to free
+   the returned value if it is nonnull and is not BUFFER.  A null
+   ALLOC stands for the standard allocator.
+
+   The PREADLINKAT function specifies how to read links.  It operates
+   like POSIX readlinkat()
+   <https://pubs.opengroup.org/onlinepubs/9699919799/functions/readlink.html>
+   but can assume that its first argument is the same as FD.
+
+   If successful, return the buffer address; otherwise return NULL and
+   set errno.  */
+
+char *
+careadlinkat (int fd, char const *filename,
+              char *buffer, size_t buffer_size,
+              struct allocator const *alloc,
+              ssize_t (*preadlinkat) (int, char const *, char *, size_t))
+{
+  /* Allocate the initial buffer on the stack.  This way, in the
+     common case of a symlink of small size, we get away with a
+     single small malloc instead of a big malloc followed by a
+     shrinking realloc.
+
+     If GCC -Wreturn-local-addr warns about this buffer, the warning
+     is bogus; see readlink_stk.  */
+  char stack_buf[STACK_BUF_SIZE];
+  return readlink_stk (fd, filename, buffer, buffer_size, alloc,
+                       preadlinkat, stack_buf);
+}