strerror_r: guarantee unchanged errno

POSIX guarantees that strerror doesn't change errno on success,
and since we implement strerror by using strerror_r, it makes
sense to make the same guarantee for strerror_r (rather, going
one step further to say that sterror_r does not corrupt errno,
since it returns an error value explicitly).

See also http://austingroupbugs.net/view.php?id=447

* lib/strerror_r.c (strerror_r): Guarantee unchanged errno.
* lib/strerror-impl.h (strerror): Set errno to match strerror_r
failure.
* tests/test-strerror_r.c (main): Enhance test.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/ChangeLog b/ChangeLog
index db41064127447d876f7b60c1c07442d60f39b927..49568395d7922008205eb7034cc49e87c37b6f1a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-05-19  Eric Blake  <eblake@redhat.com>
+
+       strerror_r: guarantee unchanged errno
+       * lib/strerror_r.c (strerror_r): Guarantee unchanged errno.
+       * lib/strerror-impl.h (strerror): Set errno to match strerror_r
+       failure.
+       * tests/test-strerror_r.c (main): Enhance test.
+
 2011-05-19  Bruno Haible  <bruno@clisp.org>
 
        strerror_r: Reorder #if blocks.

diff --git a/lib/strerror-impl.h b/lib/strerror-impl.h
index 6be2b2a8f32c8c0539206c740b9c707cb66583a8..a2042433ccc9da5a63e37c6b0b4eab15b97ec4ee 100644 (file)
--- a/lib/strerror-impl.h
+++ b/lib/strerror-impl.h
@@ -36,6 +36,7 @@ strerror (int n)
     static char const fmt[] = "Unknown error (%d)";
     verify (sizeof (buf) >= sizeof (fmt) + INT_STRLEN_BOUND (n));
     sprintf (buf, fmt, n);
+    errno = ret;
     return buf;
   }
 }

diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index db482451069ef5462ed21e4dc74f8373b3280e16..4aac345424d0a4fae5138f3094306efcbb12050f 100644 (file)
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -403,21 +403,24 @@ strerror_r (int errnum, char *buf, size_t buflen)
 
     if (msg)
       {
+        int saved_errno = errno;
         size_t len = strlen (msg);
+        int ret = ERANGE;
 
         if (len < buflen)
           {
             memcpy (buf, msg, len + 1);
-            return 0;
+            ret = 0;
           }
-        else
-          return ERANGE;
+        errno = saved_errno;
+        return ret;
       }
   }
 #endif
 
   {
     int ret;
+    int saved_errno = errno;
 
 #if USE_XPG_STRERROR_R
 
@@ -519,7 +522,6 @@ strerror_r (int errnum, char *buf, size_t buflen)
     if (errnum >= 0 && errnum < sys_nerr)
       {
 #  if HAVE_CATGETS && (defined __NetBSD__ || defined __hpux)
-        int saved_errno = errno;
 #   if defined __NetBSD__
         nl_catd catd = catopen ("libc", NL_CAT_LOCALE);
         const char *errmsg =
@@ -554,7 +556,6 @@ strerror_r (int errnum, char *buf, size_t buflen)
 #  if HAVE_CATGETS && (defined __NetBSD__ || defined __hpux)
         if (catd != (nl_catd)-1)
           catclose (catd);
-        errno = saved_errno;
 #  endif
       }
     else
@@ -618,6 +619,7 @@ strerror_r (int errnum, char *buf, size_t buflen)
 
 #endif
 
+    errno = saved_errno;
     return ret;
   }
 }

diff --git a/tests/test-strerror_r.c b/tests/test-strerror_r.c
index 7aad3c7365199e318dc7eef511fb2fc3d5cbd782..48287678bdc95de869c563878ebdcbec17cdd45b 100644 (file)
--- a/tests/test-strerror_r.c
+++ b/tests/test-strerror_r.c
@@ -34,35 +34,45 @@ main (void)
 
   /* Test results with valid errnum and enough room.  */
 
+  errno = 0;
   buf[0] = '\0';
   ASSERT (strerror_r (EACCES, buf, sizeof (buf)) == 0);
   ASSERT (buf[0] != '\0');
+  ASSERT (errno == 0);
 
+  errno = 0;
   buf[0] = '\0';
   ASSERT (strerror_r (ETIMEDOUT, buf, sizeof (buf)) == 0);
   ASSERT (buf[0] != '\0');
+  ASSERT (errno == 0);
 
+  errno = 0;
   buf[0] = '\0';
   ASSERT (strerror_r (EOVERFLOW, buf, sizeof (buf)) == 0);
   ASSERT (buf[0] != '\0');
+  ASSERT (errno == 0);
 
   /* POSIX requires strerror (0) to succeed.  Reject use of "Unknown
      error", but allow "Success", "No error", or even Solaris' "Error
      0" which are distinct patterns from true out-of-range strings.
      http://austingroupbugs.net/view.php?id=382  */
+  errno = 0;
   buf[0] = '\0';
   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.  */
 
+  errno = 0;
   buf[0] = '^';
   ret = strerror_r (-3, buf, sizeof (buf));
   ASSERT (ret == 0 || ret == EINVAL);
   if (ret == 0)
     ASSERT (buf[0] != '^');
+  ASSERT (errno == 0);
 
   /* Test results with a too small buffer.  */
 
@@ -74,7 +84,9 @@ main (void)
     for (i = 0; i <= len; i++)
       {
         strcpy (buf, "BADFACE");
+        errno = 0;
         ret = strerror_r (EACCES, buf, i);
+        ASSERT (errno == 0);
         if (ret == 0)
           {
             /* Truncated result.  POSIX allows this, and it actually
@@ -90,8 +102,10 @@ main (void)
       }
 
     strcpy (buf, "BADFACE");
+    errno = 0;
     ret = strerror_r (EACCES, buf, len + 1);
     ASSERT (ret == 0);
+    ASSERT (errno == 0);
   }
 
   return 0;