proposed new module breadlinkat

[Paul Eggert]

In seeking to have GNU Emacs use gnulib's areadlink module I
ran into a problem.  Emacs has its own xmalloc and xfree
functions that block interrupts, and it can't simply use
areadlink since areadlink uses malloc without blocking
interrupts.  One workaround would be to block interrupts for
the entire duration of the areadlink, but that would be bad
because the underlying readlink may take some time
(especially under NFS).  Better would be a variant of
areadlink that lets Emacs uses its own xmalloc, xfree, etc.

This proposed patch does that, with a new module
breadlinkat.  A companion patch rewrites areadlink and
areadlinkat to use the new breadlinkat module, thus avoiding
the duplication of code among areadlink, areadlinkat, and
breadlinkat.

Three more points.

 1.  areadlink.c does '#undef malloc', '#undef realloc' but
     areadlinkat.c does not.  Is this intended?  (This issue
     is independent of the above change.)

 2.  The areadlink-with-size module is obsolescent, now that
     areadlink, areadlinkat, nd breadlinkat read the link
     contents into stack space first (unless the link
     contents' length is 1 KiB or more, which is so rare we
     needn't worry about it for performance reasons).  I
     originally did the readlink-with-size business back
     when the code always did malloc first.  The stack-based
     performance improvement obsoletes the -with-size
     performance improvement, so I propose that we remove
     the areadlink-with-size module and replace all calls
     with calls to areadlink.

 3.  Similarly for areadlinkat-with-size.

>From b0df81c31dbf2dc603e285f126ac9b76f9d2518d Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Mon, 28 Mar 2011 20:13:05 -0700
Subject: [PATCH 1/2] breadlinkat: new module

* lib/breadlinkat.c, lib/breadlinkat.h, modules/breadlinkat:
New files.
---
 ChangeLog           |    6 ++
 lib/breadlinkat.c   |  164 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/breadlinkat.h   |   64 ++++++++++++++++++++
 modules/breadlinkat |   26 ++++++++
 4 files changed, 260 insertions(+), 0 deletions(-)
 create mode 100644 lib/breadlinkat.c
 create mode 100644 lib/breadlinkat.h
 create mode 100644 modules/breadlinkat

diff --git a/ChangeLog b/ChangeLog
index a74a0c0..af0b8e7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-03-28  Paul Eggert  <address@hidden>
+
+       breadlinkat: new module
+       * lib/breadlinkat.c, lib/breadlinkat.h, modules/breadlinkat:
+       New files.
+
 2011-03-28  Simon Josefsson  <address@hidden>
 
        * doc/posix-functions/getaddrinfo.texi: Drop netdb.h discussion.
diff --git a/lib/breadlinkat.c b/lib/breadlinkat.c
new file mode 100644
index 0000000..11da1a0
--- /dev/null
+++ b/lib/breadlinkat.c
@@ -0,0 +1,164 @@
+/* Read symbolic links into a buffer without size limitation, relative to fd.
+
+   Copyright (C) 2001, 2003-2004, 2007, 2009-2011 Free Software Foundation,
+   Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Paul Eggert, Bruno Haible, and Jim Meyering.  */
+
+#include <config.h>
+
+#include "breadlinkat.h"
+
+#include <errno.h>
+#include <limits.h>
+#include <string.h>
+#include <unistd.h>
+
+/* Define this independently so that stdint.h is not a prerequisite.  */
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
+
+#ifndef SSIZE_MAX
+# define SSIZE_MAX ((ssize_t) (SIZE_MAX / 2))
+#endif
+
+#if ! HAVE_READLINKAT
+/* Ignore FD.  Get the symbolic link value of FILENAME and put it into
+   BUFFER, with size BUFFER_SIZE.  This function acts like readlink
+   but has readlinkat's signature.  */
+ssize_t
+breadlinkatcwd (int fd, char const *filename, char *buffer, size_t buffer_size)
+{
+  (void) fd;
+  return readlink (filename, buffer, buffer_size);
+}
+#endif
+
+/* 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 PMALLOC, PREALLOC (if nonnull), and PFREE.  It is
+   the caller's responsibility to free the returned value if it is
+   nonnull and is not BUFFER.  Report any memory allocation
+   failure by calling PALLOC_DIE if nonnull.
+
+   The PREADLINKAT function specifies how to read links.
+
+   If successful, return the buffer address; otherwise return NULL and
+   set errno.  */
+
+char *
+breadlinkat (int fd, char const *filename,
+             char *buffer, size_t buffer_size,
+             void *(*pmalloc) (size_t),
+             void *(*prealloc) (void *, size_t),
+             void (*pfree) (void *),
+             void (*palloc_die) (void),
+             ssize_t (*preadlinkat) (int, char const *, char *, size_t))
+{
+  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 (! buffer_size)
+    {
+      /* 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;
+      buffer_size = sizeof stack_buf;
+    }
+
+  buf = buffer;
+  buf_size = buffer_size;
+
+  while (1)
+    {
+      /* Attempt to read the link into the current buffer.  */
+      ssize_t link_length = preadlinkat (fd, filename, buf, buf_size);
+      size_t link_size;
+      if (link_length < 0)
+        {
+          /* On AIX 5L v5.3 and HP-UX 11i v2 04/09, readlink returns -1
+             with errno == ERANGE if the buffer is too small.  */
+          int readlinkat_errno = errno;
+          if (readlinkat_errno != ERANGE)
+            {
+              if (buf != buffer)
+                {
+                  pfree (buf);
+                  errno = readlinkat_errno;
+                }
+              return NULL;
+            }
+        }
+
+      link_size = link_length;
+
+      if (link_size < buf_size)
+        {
+          buf[link_size++] = '\0';
+
+          if (buf == stack_buf)
+            {
+              char *b = (char *) pmalloc (link_size);
+              if (! b)
+                break;
+              memcpy (b, buf, link_size);
+              buf = b;
+            }
+          else if (link_size < buf_size && buf != buffer && prealloc)
+            {
+              /* Shrink BUF before returning it.  */
+              char *b = (char *) prealloc (buf, link_size);
+              if (b)
+                buf = b;
+            }
+
+          return buf;
+        }
+
+      if (buf != buffer)
+        pfree (buf);
+
+      if (buf_size <= buf_size_max / 2)
+        buf_size *= 2;
+      else if (buf_size < buf_size_max)
+        buf_size = buf_size_max;
+      else
+        break;
+      buf = (char *) pmalloc (buf_size);
+      if (! buf)
+        break;
+    }
+
+
+  if (palloc_die)
+    palloc_die ();
+  errno = ENOMEM;
+  return NULL;
+}
diff --git a/lib/breadlinkat.h b/lib/breadlinkat.h
new file mode 100644
index 0000000..6381a91
--- /dev/null
+++ b/lib/breadlinkat.h
@@ -0,0 +1,64 @@
+/* Read symbolic links into a buffer without size limitation, relative to fd.
+
+   Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Paul Eggert, Bruno Haible, and Jim Meyering.  */
+
+#ifndef _GL_BREADLINKAT_H
+
+#include <unistd.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 PMALLOC, PREALLOC (if nonnull), and PFREE.  It is
+   the caller's responsibility to free the returned value if it is
+   nonnull and is not BUFFER.  Report any memory allocation
+   failure by calling PALLOC_DIE if nonnull.
+
+   The PREADLINKAT function specifies how to read links.
+
+   If successful, return the buffer address; otherwise return NULL and
+   set errno.  */
+
+char *breadlinkat (int fd, char const *filename,
+                   char *buffer, size_t buffer_size,
+                   void *(*pmalloc) (size_t),
+                   void *(*prealloc) (void *, size_t),
+                   void (*pfree) (void *),
+                   void (*palloc_die) (void),
+                   ssize_t (*preadlinkat) (int, char const *, char *, size_t));
+
+/* Suitable values for breadlinkat's FD and PREADLINKAT arguments,
+   when doing a plain readlink.  */
+#if HAVE_READLINKAT
+# include <fcntl.h>
+# define BREADLINKAT_FDCWD AT_FDCWD
+# define breadlinkatcwd readlinkat
+#else
+# define BREADLINKAT_FDCWD 0 /* Ignored by breadlinkatcwd.  */
+ssize_t breadlinkatcwd (int fd, char const *filename,
+                        char *buffer, size_t buffer_size);
+#endif
+
+#endif /* _GL_BREADLINKAT_H */
diff --git a/modules/breadlinkat b/modules/breadlinkat
new file mode 100644
index 0000000..1678eac
--- /dev/null
+++ b/modules/breadlinkat
@@ -0,0 +1,26 @@
+Description:
+Read symbolic links into a buffer without size limitation, relative to fd.
+
+Files:
+lib/breadlinkat.c
+lib/breadlinkat.h
+
+Depends-on:
+readlink
+ssize_t
+unistd
+
+configure.ac:
+AC_CHECK_FUNCS_ONCE([readlinkat])
+
+Makefile.am:
+lib_SOURCES += breadlinkat.c
+
+Include:
+"breadlinkat.h"
+
+License:
+LGPLv2+
+
+Maintainer:
+Paul Eggert, Bruno Haible, Jim Meyering
-- 
1.7.4


>From 6a29e3f42dd769d478e40000707947e14281c1b0 Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Mon, 28 Mar 2011 20:49:46 -0700
Subject: [PATCH 2/2] areadlink, areadlinkat: rewrite in terms of breadlinkat

* lib/areadlink.c, lib/areadlinkat.c: Include breadlinkat.h
instead of errno.h, limits.h, stdint.h, string.h, unistd.h.
(SSIZE_MAX, INITIAL_BUF_SIZE): Remove.
(areadlink, areadlinkat): Rewrite in terms of breadlinkat.
* modules/areadlink, modules/areadlinkat (Depends-on): Add breadlinkat.
Remove areadlink, stdint.
* modules/areadlink (Depends-on): Remove readlink, ssize_t.
---
 ChangeLog           |    9 +++++
 lib/areadlink.c     |   95 +++------------------------------------------------
 lib/areadlinkat.c   |   87 +++--------------------------------------------
 modules/areadlink   |    5 +--
 modules/areadlinkat |    3 +-
 5 files changed, 21 insertions(+), 178 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index af0b8e7..824e9ca 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2011-03-28  Paul Eggert  <address@hidden>
 
+       areadlink, areadlinkat: rewrite in terms of breadlinkat
+       * lib/areadlink.c, lib/areadlinkat.c: Include breadlinkat.h
+       instead of errno.h, limits.h, stdint.h, string.h, unistd.h.
+       (SSIZE_MAX, INITIAL_BUF_SIZE): Remove.
+       (areadlink, areadlinkat): Rewrite in terms of breadlinkat.
+       * modules/areadlink, modules/areadlinkat (Depends-on): Add breadlinkat.
+       Remove areadlink, stdint.
+       * modules/areadlink (Depends-on): Remove readlink, ssize_t.
+
        breadlinkat: new module
        * lib/breadlinkat.c, lib/breadlinkat.h, modules/breadlinkat:
        New files.
diff --git a/lib/areadlink.c b/lib/areadlink.c
index bc6104f..6db9063 100644
--- a/lib/areadlink.c
+++ b/lib/areadlink.c
@@ -24,108 +24,23 @@
 /* Specification.  */
 #include "areadlink.h"
 
-#include <errno.h>
-#include <limits.h>
-#include <stdint.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
+#include "breadlinkat.h"
 
-#ifndef SSIZE_MAX
-# define SSIZE_MAX ((ssize_t) (SIZE_MAX / 2))
-#endif
+#include <stdlib.h>
 
 /* Use the system functions, not the gnulib overrides in this file.  */
 #undef malloc
 #undef realloc
 
-/* The initial buffer size for the link value.  A power of 2
-   detects arithmetic overflow earlier, but is not required.  */
-enum {
-  INITIAL_BUF_SIZE = 1024
-};
-
 /* Call readlink to get the symbolic link value of FILENAME.
    Return a pointer to that NUL-terminated string in malloc'd storage.
    If readlink fails, return NULL and set errno.
-   If realloc fails, or if the link value is longer than SIZE_MAX :-),
+   If allocation fails, or if the link value is longer than SIZE_MAX :-),
    return NULL and set errno to ENOMEM.  */
 
 char *
 areadlink (char const *filename)
 {
-  /* 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().  */
-  char initial_buf[INITIAL_BUF_SIZE];
-
-  char *buffer = initial_buf;
-  size_t buf_size = sizeof initial_buf;
-
-  while (1)
-    {
-      /* Attempt to read the link into the current buffer.  */
-      ssize_t link_length = readlink (filename, buffer, buf_size);
-
-      /* On AIX 5L v5.3 and HP-UX 11i v2 04/09, readlink returns -1
-         with errno == ERANGE if the buffer is too small.  */
-      if (link_length < 0 && errno != ERANGE)
-        {
-          if (buffer != initial_buf)
-            {
-              int saved_errno = errno;
-              free (buffer);
-              errno = saved_errno;
-            }
-          return NULL;
-        }
-
-      if ((size_t) link_length < buf_size)
-        {
-          buffer[link_length++] = '\0';
-
-          /* Return it in a chunk of memory as small as possible.  */
-          if (buffer == initial_buf)
-            {
-              buffer = (char *) malloc (link_length);
-              if (buffer == NULL)
-                {
-                  /* It's easier to set errno to ENOMEM than to rely on the
-                     'malloc-posix' gnulib module.  */
-                  errno = ENOMEM;
-                  return NULL;
-                }
-              memcpy (buffer, initial_buf, link_length);
-            }
-          else
-            {
-              /* Shrink buffer before returning it.  */
-              if ((size_t) link_length < buf_size)
-                {
-                  char *smaller_buffer = (char *) realloc (buffer, 
link_length);
-
-                  if (smaller_buffer != NULL)
-                    buffer = smaller_buffer;
-                }
-            }
-          return buffer;
-        }
-
-      if (buffer != initial_buf)
-        free (buffer);
-      buf_size *= 2;
-      if (SSIZE_MAX < buf_size || (SIZE_MAX / 2 < SSIZE_MAX && buf_size == 0))
-        {
-          errno = ENOMEM;
-          return NULL;
-        }
-      buffer = (char *) malloc (buf_size);
-      if (buffer == NULL)
-        {
-          /* It's easier to set errno to ENOMEM than to rely on the
-             'malloc-posix' gnulib module.  */
-          errno = ENOMEM;
-          return NULL;
-        }
-    }
+  return breadlinkat (BREADLINKAT_FDCWD, filename, NULL, 0,
+                      malloc, realloc, free, NULL, breadlinkatcwd);
 }
diff --git a/lib/areadlinkat.c b/lib/areadlinkat.c
index a13c0e5..300e1ea 100644
--- a/lib/areadlinkat.c
+++ b/lib/areadlinkat.c
@@ -25,101 +25,24 @@
 /* Specification.  */
 #include "areadlink.h"
 
-#include <errno.h>
-#include <limits.h>
-#include <stdint.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
+#include "breadlinkat.h"
 
-#ifndef SSIZE_MAX
-# define SSIZE_MAX ((ssize_t) (SIZE_MAX / 2))
-#endif
+#include <stdlib.h>
 
 #if HAVE_READLINKAT
 
-/* The initial buffer size for the link value.  A power of 2
-   detects arithmetic overflow earlier, but is not required.  */
-enum {
-  INITIAL_BUF_SIZE = 1024
-};
-
 /* Call readlinkat to get the symbolic link value of FILENAME relative to FD.
    Return a pointer to that NUL-terminated string in malloc'd storage.
    If readlinkat fails, return NULL and set errno (although failure to
    change directory will issue a diagnostic and exit).
-   If realloc fails, or if the link value is longer than SIZE_MAX :-),
+   If allocation fails, or if the link value is longer than SIZE_MAX :-),
    return NULL and set errno to ENOMEM.  */
 
 char *
 areadlinkat (int fd, char const *filename)
 {
-  /* 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().  */
-  char initial_buf[INITIAL_BUF_SIZE];
-
-  char *buffer = initial_buf;
-  size_t buf_size = sizeof initial_buf;
-
-  while (1)
-    {
-      /* Attempt to read the link into the current buffer.  */
-       ssize_t link_length = readlinkat (fd, filename, buffer, buf_size);
-
-      /* On AIX 5L v5.3 and HP-UX 11i v2 04/09, readlink returns -1
-         with errno == ERANGE if the buffer is too small.  */
-      if (link_length < 0 && errno != ERANGE)
-        {
-          if (buffer != initial_buf)
-            {
-              int saved_errno = errno;
-              free (buffer);
-              errno = saved_errno;
-            }
-          return NULL;
-        }
-
-      if ((size_t) link_length < buf_size)
-        {
-          buffer[link_length++] = '\0';
-
-          /* Return it in a chunk of memory as small as possible.  */
-          if (buffer == initial_buf)
-            {
-              buffer = (char *) malloc (link_length);
-              if (buffer == NULL)
-                /* errno is ENOMEM.  */
-                return NULL;
-              memcpy (buffer, initial_buf, link_length);
-            }
-          else
-            {
-              /* Shrink buffer before returning it.  */
-              if ((size_t) link_length < buf_size)
-                {
-                  char *smaller_buffer = (char *) realloc (buffer, 
link_length);
-
-                  if (smaller_buffer != NULL)
-                    buffer = smaller_buffer;
-                }
-            }
-          return buffer;
-        }
-
-      if (buffer != initial_buf)
-        free (buffer);
-      buf_size *= 2;
-      if (SSIZE_MAX < buf_size || (SIZE_MAX / 2 < SSIZE_MAX && buf_size == 0))
-        {
-          errno = ENOMEM;
-          return NULL;
-        }
-      buffer = (char *) malloc (buf_size);
-      if (buffer == NULL)
-        /* errno is ENOMEM.  */
-        return NULL;
-    }
+  return breadlinkat (fd, filename, NULL, 0,
+                      malloc, realloc, free, NULL, readlinkat);
 }
 
 #else /* !HAVE_READLINKAT */
diff --git a/modules/areadlink b/modules/areadlink
index 3166269..36cddf7 100644
--- a/modules/areadlink
+++ b/modules/areadlink
@@ -6,10 +6,7 @@ lib/areadlink.h
 lib/areadlink.c
 
 Depends-on:
-readlink
-ssize_t
-stdint
-unistd
+breadlinkat
 
 configure.ac:
 
diff --git a/modules/areadlinkat b/modules/areadlinkat
index 072f823..f72e6c9 100644
--- a/modules/areadlinkat
+++ b/modules/areadlinkat
@@ -6,8 +6,7 @@ lib/areadlink.h
 lib/areadlinkat.c
 
 Depends-on:
-areadlink
-stdint
+breadlinkat
 readlinkat
 
 configure.ac:
-- 
1.7.4