Mercurial > gnulib

changeset 34506:47292526799e

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
strerror_r: enforce POSIX recommendations POSIX recommends (but does not require) that strerror_r populate buf even on error. But since we guarantee this behavior for strerror, we might as well also guarantee it for strerror_r. * lib/strerror_r.c (safe_copy): New helper method. (strerror_r): Guarantee a non-empty string. * tests/test-strerror_r.c (main): Enhance tests to incorporate recent POSIX rulings and to match our strerror guarantees. * doc/posix-functions/strerror_r.texi (strerror_r): Document this. Signed-off-by: Eric Blake <eblake@redhat.com>
author Eric Blake <eblake@redhat.com>
date Wed, 18 May 2011 15:19:51 -0600
parents 6a977ceedea0
children c9ef36ab30e8
files ChangeLog doc/posix-functions/strerror_r.texi lib/strerror_r.c tests/test-strerror_r.c
diffstat 4 files changed, 118 insertions(+), 111 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Tue May 24 21:28:46 2011 +0200
+++ b/ChangeLog	Wed May 18 15:19:51 2011 -0600
@@ -1,3 +1,12 @@
+2011-05-24  Eric Blake  <eblake@redhat.com>
+
+	strerror_r: enforce POSIX recommendations
+	* lib/strerror_r.c (safe_copy): New helper method.
+	(strerror_r): Guarantee a non-empty string.
+	* tests/test-strerror_r.c (main): Enhance tests to incorporate
+	recent POSIX rulings and to match our strerror guarantees.
+	* doc/posix-functions/strerror_r.texi (strerror_r): Document this.
+
 2011-05-24  Jim Meyering  <meyering@redhat.com>
 
 	test-perror2.c: avoid warning about unused variable
--- a/doc/posix-functions/strerror_r.texi	Tue May 24 21:28:46 2011 +0200
+++ b/doc/posix-functions/strerror_r.texi	Wed May 18 15:19:51 2011 -0600
@@ -51,15 +51,21 @@
 platforms:
 HP-UX 11.31.
 @item
-When the buffer is too small, this function does not fail, but instead
-truncates the result and returns 0 on some platforms:
-OSF/1 5.1.
+When the buffer is too small and the value is in range, this function
+does not fail, but instead truncates the result and returns 0 on some
+platforms:
+AIX 6.1, OSF/1 5.1.
+@item
+When the value is not in range or the buffer is too small, this
+function fails to leave a NUL-terminated string in the buffer on some
+platforms:
+glibc 2.13, FreeBSD 8.2.
 @end itemize
 
 Portability problems not fixed by Gnulib:
 @itemize
 @item
-When the buffer is too small, this function does not fail, but instead
-truncates the result and returns 0 on some platforms:
-AIX 6.1.
+Calling this function can clobber the buffer used by @code{strerror}
+on some platforms:
+Cygwin 1.7.9.
 @end itemize
--- a/lib/strerror_r.c	Tue May 24 21:28:46 2011 +0200
+++ b/lib/strerror_r.c	Wed May 18 15:19:51 2011 -0600
@@ -92,6 +92,30 @@
 
 #endif
 
+/* Copy as much of MSG into BUF as possible, without corrupting errno.
+   Return 0 if MSG fit in BUFLEN, otherwise return ERANGE.  */
+static int
+safe_copy (char *buf, size_t buflen, const char *msg)
+{
+  size_t len = strlen (msg);
+  int ret;
+
+  if (len < buflen)
+    {
+      /* Although POSIX allows memcpy() to corrupt errno, we don't
+         know of any implementation where this is a real problem.  */
+      memcpy (buf, msg, len + 1);
+      ret = 0;
+    }
+  else
+    {
+      memcpy (buf, msg, buflen - 1);
+      buf[buflen - 1] = '\0';
+      ret = ERANGE;
+    }
+  return ret;
+}
+
 
 int
 strerror_r (int errnum, char *buf, size_t buflen)
@@ -102,9 +126,10 @@
   if (buflen <= 1)
     {
       if (buflen)
-        *buf = 0;
+        *buf = '\0';
       return ERANGE;
     }
+  *buf = '\0';
 
 #if GNULIB_defined_ETXTBSY \
     || GNULIB_defined_ESOCK \
@@ -413,19 +438,7 @@
       }
 
     if (msg)
-      {
-        int saved_errno = errno;
-        size_t len = strlen (msg);
-        int ret = ERANGE;
-
-        if (len < buflen)
-          {
-            memcpy (buf, msg, len + 1);
-            ret = 0;
-          }
-        errno = saved_errno;
-        return ret;
-      }
+      return safe_copy (buf, buflen, msg);
   }
 #endif
 
@@ -441,6 +454,13 @@
       ret = __xpg_strerror_r (errnum, buf, buflen);
       if (ret < 0)
         ret = errno;
+      if (!*buf)
+        {
+          /* glibc 2.13 would not touch buf on err, so we have to fall
+             back to GNU strerror_r which always returns a thread-safe
+             untruncated string to (partially) copy into our buf.  */
+          safe_copy (buf, buflen, strerror_r (errnum, buf, buflen));
+        }
     }
 
 #elif USE_SYSTEM_STRERROR_R
@@ -453,18 +473,11 @@
     {
       char stackbuf[80];
 
-      if (buflen < sizeof (stackbuf))
+      if (buflen < sizeof stackbuf)
         {
-          ret = strerror_r (errnum, stackbuf, sizeof (stackbuf));
+          ret = strerror_r (errnum, stackbuf, sizeof stackbuf);
           if (ret == 0)
-            {
-              size_t len = strlen (stackbuf);
-
-              if (len < buflen)
-                memcpy (buf, stackbuf, len + 1);
-              else
-                ret = ERANGE;
-            }
+            ret = safe_copy (buf, buflen, stackbuf);
         }
       else
         ret = strerror_r (errnum, buf, buflen);
@@ -479,19 +492,7 @@
 
     /* FreeBSD rejects 0; see http://austingroupbugs.net/view.php?id=382.  */
     if (errnum == 0 && ret == EINVAL)
-      {
-        if (buflen <= strlen ("Success"))
-          {
-            ret = ERANGE;
-            if (buflen)
-              buf[0] = 0;
-          }
-        else
-          {
-            ret = 0;
-            strcpy (buf, "Success");
-          }
-      }
+      ret = safe_copy (buf, buflen, "Success");
 
 #else /* USE_SYSTEM_STRERROR */
 
@@ -528,17 +529,7 @@
         if (errmsg == NULL || *errmsg == '\0')
           ret = EINVAL;
         else
-          {
-            size_t len = strlen (errmsg);
-
-            if (len < buflen)
-              {
-                memcpy (buf, errmsg, len + 1);
-                ret = 0;
-              }
-            else
-              ret = ERANGE;
-          }
+          ret = safe_copy (buf, buflen, errmsg);
 #  if HAVE_CATGETS && (defined __NetBSD__ || defined __hpux)
         if (catd != (nl_catd)-1)
           catclose (catd);
@@ -558,17 +549,7 @@
         if (errmsg == NULL || *errmsg == '\0')
           ret = EINVAL;
         else
-          {
-            size_t len = strlen (errmsg);
-
-            if (len < buflen)
-              {
-                memcpy (buf, errmsg, len + 1);
-                ret = 0;
-              }
-            else
-              ret = ERANGE;
-          }
+          ret = safe_copy (buf, buflen, errmsg);
       }
     else
       ret = EINVAL;
@@ -586,17 +567,7 @@
       if (errmsg == NULL || *errmsg == '\0')
         ret = EINVAL;
       else
-        {
-          size_t len = strlen (errmsg);
-
-          if (len < buflen)
-            {
-              memcpy (buf, errmsg, len + 1);
-              ret = 0;
-            }
-          else
-            ret = ERANGE;
-        }
+        ret = safe_copy (buf, buflen, errmsg);
     }
 
     gl_lock_unlock (strerror_lock);
@@ -605,6 +576,9 @@
 
 #endif
 
+    if (ret == EINVAL && !*buf)
+      snprintf (buf, buflen, "Unknown error %d", errnum);
+
     errno = saved_errno;
     return ret;
   }
--- a/tests/test-strerror_r.c	Tue May 24 21:28:46 2011 +0200
+++ b/tests/test-strerror_r.c	Wed May 18 15:19:51 2011 -0600
@@ -36,21 +36,24 @@
 
   errno = 0;
   buf[0] = '\0';
-  ASSERT (strerror_r (EACCES, buf, sizeof (buf)) == 0);
+  ASSERT (strerror_r (EACCES, buf, sizeof buf) == 0);
   ASSERT (buf[0] != '\0');
   ASSERT (errno == 0);
+  ASSERT (strlen (buf) < sizeof buf);
 
   errno = 0;
   buf[0] = '\0';
-  ASSERT (strerror_r (ETIMEDOUT, buf, sizeof (buf)) == 0);
+  ASSERT (strerror_r (ETIMEDOUT, buf, sizeof buf) == 0);
   ASSERT (buf[0] != '\0');
   ASSERT (errno == 0);
+  ASSERT (strlen (buf) < sizeof buf);
 
   errno = 0;
   buf[0] = '\0';
-  ASSERT (strerror_r (EOVERFLOW, buf, sizeof (buf)) == 0);
+  ASSERT (strerror_r (EOVERFLOW, buf, sizeof buf) == 0);
   ASSERT (buf[0] != '\0');
   ASSERT (errno == 0);
+  ASSERT (strlen (buf) < sizeof buf);
 
   /* POSIX requires strerror (0) to succeed.  Reject use of "Unknown
      error", but allow "Success", "No error", or even Solaris' "Error
@@ -58,54 +61,69 @@
      http://austingroupbugs.net/view.php?id=382  */
   errno = 0;
   buf[0] = '\0';
-  ret = strerror_r (0, buf, sizeof (buf));
+  ret = strerror_r (0, buf, sizeof buf);
   ASSERT (ret == 0);
   ASSERT (buf[0]);
   ASSERT (errno == 0);
   ASSERT (strstr (buf, "nknown") == NULL);
 
-  /* Test results with out-of-range errnum and enough room.  */
-
+  /* Test results with out-of-range errnum and enough room.  POSIX
+     allows an empty string on success, and allows an unchanged buf on
+     error, but these are not useful, so we guarantee contents.  */
   errno = 0;
   buf[0] = '^';
-  ret = strerror_r (-3, buf, sizeof (buf));
+  ret = strerror_r (-3, buf, sizeof buf);
   ASSERT (ret == 0 || ret == EINVAL);
-  if (ret == 0)
-    ASSERT (buf[0] != '^');
+  ASSERT (buf[0] != '^');
+  ASSERT (*buf);
   ASSERT (errno == 0);
+  ASSERT (strlen (buf) < sizeof buf);
 
-  /* Test results with a too small buffer.  */
+  /* Test results with a too small buffer.  POSIX requires an error;
+     only ERANGE for 0 and valid errors, and a choice of ERANGE or
+     EINVAL for out-of-range values.  On error, POSIX permits buf to
+     be empty, unchanged, or unterminated, but these are not useful,
+     so we guarantee NUL-terminated truncated contents for all but
+     size 0.  http://austingroupbugs.net/view.php?id=398  */
+  {
+    int errs[] = { EACCES, 0, -3, };
+    int j;
+    for (j = 0; j < SIZEOF (errs); j++)
+      {
+        int err = errs[j];
+        char buf2[sizeof buf] = "";
+        size_t len;
+        size_t i;
 
-  ASSERT (strerror_r (EACCES, buf, sizeof (buf)) == 0);
-  {
-    size_t len = strlen (buf);
-    size_t i;
+        strerror_r (err, buf2, sizeof buf2);
+        len = strlen (buf2);
 
-    for (i = 0; i <= len; i++)
-      {
+        for (i = 0; i <= len; i++)
+          {
+            strcpy (buf, "BADFACE");
+            errno = 0;
+            ret = strerror_r (err, buf, i);
+            ASSERT (errno == 0);
+            if (err < 0)
+              ASSERT (ret == ERANGE || ret == EINVAL);
+            else
+              ASSERT (ret == ERANGE);
+            if (i == 0)
+              ASSERT (strcmp (buf, "BADFACE") == 0);
+            else
+              {
+                ASSERT (strncmp (buf, buf2, i - 1) == 0);
+                ASSERT (buf[i - 1] == '\0');
+              }
+          }
+
         strcpy (buf, "BADFACE");
         errno = 0;
-        ret = strerror_r (EACCES, buf, i);
+        ret = strerror_r (err, buf, len + 1);
+        ASSERT (ret != ERANGE);
         ASSERT (errno == 0);
-        if (ret == 0)
-          {
-            /* Truncated result.  POSIX allows this, and it actually
-               happens on AIX 6.1 and Cygwin.  */
-            ASSERT ((strcmp (buf, "BADFACE") == 0) == (i == 0));
-          }
-        else
-          {
-            /* Failure.  */
-            ASSERT (ret == ERANGE);
-            /* buf is clobbered nevertheless, on FreeBSD and MacOS X.  */
-          }
+        ASSERT (strcmp (buf, buf2) == 0);
       }
-
-    strcpy (buf, "BADFACE");
-    errno = 0;
-    ret = strerror_r (EACCES, buf, len + 1);
-    ASSERT (ret == 0);
-    ASSERT (errno == 0);
   }
 
 #if GNULIB_STRERROR