[PATCH] strerror_r-posix: fix on MacOS

[Eric Blake]

MacOS X 10.5 strerror(0) is "Unknown error: 0", which is not distinguished
from "Unknown error: -1" for out-of-range.  Worse, strerror_r(0,,)
is "Undefined error: 0", although strerror_r for all other out-of-range
values matches strerror.

* m4/strerror.m4 (gl_FUNC_STRERROR): Flush out MacOS bug.
* m4/strerror_r.m4 (gl_FUNC_STRERROR_R_WORKS): Likewise, and fix
logic bug.
* lib/strerror_r.c (strerror_r): Fix the bug.
* lib/strerror.c (strerror): Likewise.
* doc/posix-functions/strerror_r.texi (strerror_r): Document the
problem.
* doc/posix-functions/strerror.texi (strerror): Likewise.
* doc/posix-functions/perror.texi (perror): Likewise.
* tests/test-strerror.c (main): Enhance test.
* tests/test-strerror_r.c (main): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---

Another platform made sane.

 ChangeLog                           |   15 +++++++++++++++
 doc/posix-functions/perror.texi     |    2 +-
 doc/posix-functions/strerror.texi   |    9 +++++----
 doc/posix-functions/strerror_r.texi |    6 +++++-
 lib/strerror.c                      |   10 +++++++---
 lib/strerror_r.c                    |   13 ++++++++++---
 m4/strerror.m4                      |    8 ++++++--
 m4/strerror_r.m4                    |   15 +++++++++++----
 tests/test-strerror.c               |    1 +
 tests/test-strerror_r.c             |   14 +++++++++-----
 10 files changed, 70 insertions(+), 23 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9453f3b..6cd8757 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2011-06-08  Eric Blake  <address@hidden>
+
+       strerror_r-posix: fix on MacOS
+       * m4/strerror.m4 (gl_FUNC_STRERROR): Flush out MacOS bug.
+       * m4/strerror_r.m4 (gl_FUNC_STRERROR_R_WORKS): Likewise, and fix
+       logic bug.
+       * lib/strerror_r.c (strerror_r): Fix the bug.
+       * lib/strerror.c (strerror): Likewise.
+       * doc/posix-functions/strerror_r.texi (strerror_r): Document the
+       problem.
+       * doc/posix-functions/strerror.texi (strerror): Likewise.
+       * doc/posix-functions/perror.texi (perror): Likewise.
+       * tests/test-strerror.c (main): Enhance test.
+       * tests/test-strerror_r.c (main): Likewise.
+
 2011-06-08  Bruno Haible  <address@hidden>

        gnulib-tool: Better isolation between different gnulib-tool invocations.
diff --git a/doc/posix-functions/perror.texi b/doc/posix-functions/perror.texi
index ddc7c9b..c11d2e6 100644
--- a/doc/posix-functions/perror.texi
+++ b/doc/posix-functions/perror.texi
@@ -15,7 +15,7 @@ perror
 @item
 This function treats @code{errno} of 0 like failure, although POSIX
 requires that the message declare it as a success, on some platforms:
-FreeBSD 8.2
+FreeBSD 8.2, MacOS X 10.5.
 @item
 This function clobbers the @code{strerror} buffer on some platforms:
 Cygwin 1.7.9.
diff --git a/doc/posix-functions/strerror.texi 
b/doc/posix-functions/strerror.texi
index 6f9519a..931ab4f 100644
--- a/doc/posix-functions/strerror.texi
+++ b/doc/posix-functions/strerror.texi
@@ -13,10 +13,11 @@ strerror
 but not defined by the system, on some platforms:
 OpenBSD 4.0, OSF/1 5.1, NonStop Kernel, Cygwin 1.5.x, mingw.
 @item
-This function reports failure (by setting @code{errno}) for
address@hidden(0)}, although POSIX requires this to leave @code{errno}
-unchanged and report success, on some platforms:
-FreeBSD 8.2
+This function reports failure for @code{strerror(0)} (by setting
address@hidden or using a string similar to out-of-range values),
+although POSIX requires this to leave @code{errno} unchanged and
+report success, on some platforms:
+FreeBSD 8.2, MacOS X 10.5.
 @item
 This function fails to return a string for out-of-range integers on
 some platforms:
diff --git a/doc/posix-functions/strerror_r.texi 
b/doc/posix-functions/strerror_r.texi
index 4169ca9..5aaa1c6 100644
--- a/doc/posix-functions/strerror_r.texi
+++ b/doc/posix-functions/strerror_r.texi
@@ -45,7 +45,11 @@ strerror_r
 @item
 This function reports failure for @code{strerror_r(0, buf, len)},
 although POSIX requires this to succeed, on some platforms:
-FreeBSD 8.2
+FreeBSD 8.2.
address@hidden
+This function produces a different string for @code{0} than
address@hidden on some platforms:
+MacOS X 10.5.
 @item
 This function always fails when the third argument is less than 80 on some
 platforms:
diff --git a/lib/strerror.c b/lib/strerror.c
index 4dc0b65..d0dd1af 100644
--- a/lib/strerror.c
+++ b/lib/strerror.c
@@ -45,7 +45,8 @@ strerror (int n)
   if (msg)
     return (char *) msg;

-  /* FreeBSD rejects 0; see http://austingroupbugs.net/view.php?id=382.  */
+  /* FreeBSD rejects 0; see http://austingroupbugs.net/view.php?id=382.
+     MacOS X 10.5 does not distinguish 0 from -1.  */
   if (n)
     msg = strerror (n);
   else
@@ -53,14 +54,17 @@ strerror (int n)
       int saved_errno = errno;
       errno = 0;
       msg = strerror (n);
-      if (errno)
+      if (errno || (msg &&
+                    (strstr (msg, "nknown") || strstr (msg, "ndefined"))))
         msg = "Success";
       errno = saved_errno;
     }

   /* Our strerror_r implementation might use the system's strerror
      buffer, so all other clients of strerror have to see the error
-     copied into a buffer that we manage.  */
+     copied into a buffer that we manage.  This is not thread-safe,
+     even if the system strerror is, but portable programs shouldn't
+     be using strerror if they care about thread-safety.  */
   if (!msg || !*msg)
     {
       static char const fmt[] = "Unknown error %d";
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index d0c7be9..46b47ed 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -209,9 +209,16 @@ strerror_r (int errnum, char *buf, size_t buflen)
     if (ret < 0)
       ret = errno;

-    /* FreeBSD rejects 0; see http://austingroupbugs.net/view.php?id=382.  */
-    if (errnum == 0 && ret == EINVAL)
-      ret = safe_copy (buf, buflen, "Success");
+    /* FreeBSD rejects 0; see http://austingroupbugs.net/view.php?id=382.
+       MacOS X 10.5 strerror_r differs from the strerror string for 0.  */
+    if (errnum == 0)
+      {
+# if defined __APPLE__ && defined __MACH__
+        ret = EINVAL;
+# endif
+        if (ret == EINVAL)
+          ret = safe_copy (buf, buflen, "Success");
+      }

 #else /* USE_SYSTEM_STRERROR */

diff --git a/m4/strerror.m4 b/m4/strerror.m4
index 048b03c..03a0b1a 100644
--- a/m4/strerror.m4
+++ b/m4/strerror.m4
@@ -1,4 +1,4 @@
-# strerror.m4 serial 14
+# strerror.m4 serial 15
 dnl Copyright (C) 2002, 2007-2011 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -20,10 +20,14 @@ AC_DEFUN([gl_FUNC_STRERROR],
              #include <errno.h>
            ]],
            [[int result = 0;
+             char *str;
              if (!*strerror (-2)) result |= 1;
              errno = 0;
-             if (!*strerror (0)) result |= 2;
+             str = strerror (0);
+             if (!*str) result |= 2;
              if (errno) result |= 4;
+             if (strstr (str, "nknown") || strstr (str, "ndefined"))
+               result |= 8;
              return result;]])],
         [gl_cv_func_working_strerror=yes],
         [gl_cv_func_working_strerror=no],
diff --git a/m4/strerror_r.m4 b/m4/strerror_r.m4
index 68a0b69..8975827 100644
--- a/m4/strerror_r.m4
+++ b/m4/strerror_r.m4
@@ -1,4 +1,4 @@
-# strerror_r.m4 serial 10
+# strerror_r.m4 serial 11
 dnl Copyright (C) 2002, 2007-2011 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -74,6 +74,7 @@ AC_DEFUN([gl_FUNC_STRERROR_R_WORKS],
         dnl HP-UX 11.31 strerror_r always fails when the buffer length argument
         dnl is less than 80.
         dnl FreeBSD 8.s strerror_r claims failure on 0
+        dnl MacOS X 10.5 strerror_r treats 0 like -1
         dnl Solaris 10 strerror_r corrupts errno on failure
         AC_CACHE_CHECK([whether strerror_r works],
           [gl_cv_func_strerror_r_works],
@@ -89,15 +90,21 @@ AC_DEFUN([gl_FUNC_STRERROR_R_WORKS],
                   errno = 0;
                   if (strerror_r (EACCES, buf, sizeof buf) != 0)
                     result |= 2;
+                  strcpy (buf, "Unknown");
                   if (strerror_r (0, buf, sizeof buf) != 0)
                     result |= 4;
                   if (errno)
                     result |= 8;
-                  errno = 0;
-                  if (strerror_r (-3, buf, sizeof buf) != 0)
+                  if (strstr (buf, "nknown") || strstr (buf, "ndefined"))
                     result |= 0x10;
-                  if (errno)
+                  errno = 0;
+                  *buf = 0;
+                  if (strerror_r (-3, buf, sizeof buf) < 0)
                     result |= 0x20;
+                  if (errno)
+                    result |= 0x40;
+                  if (!*buf)
+                    result |= 0x80;
                   return result;
                 ]])],
              [gl_cv_func_strerror_r_works=yes],
diff --git a/tests/test-strerror.c b/tests/test-strerror.c
index 03637d7..3ffb12e 100644
--- a/tests/test-strerror.c
+++ b/tests/test-strerror.c
@@ -61,6 +61,7 @@ main (void)
   ASSERT (*str);
   ASSERT (errno == 0);
   ASSERT (strstr (str, "nknown") == NULL);
+  ASSERT (strstr (str, "ndefined") == NULL);

   /* POSIX requires strerror to produce a non-NULL result for all
      inputs; as an extension, we also guarantee a non-empty reseult.
diff --git a/tests/test-strerror_r.c b/tests/test-strerror_r.c
index 1f23ba8..956c458 100644
--- a/tests/test-strerror_r.c
+++ b/tests/test-strerror_r.c
@@ -66,6 +66,7 @@ main (void)
   ASSERT (buf[0]);
   ASSERT (errno == 0);
   ASSERT (strstr (buf, "nknown") == NULL);
+  ASSERT (strstr (buf, "ndefined") == NULL);

   /* Test results with out-of-range errnum and enough room.  POSIX
      allows an empty string on success, and allows an unchanged buf on
@@ -84,10 +85,13 @@ main (void)
      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  */
+     size 0.  http://austingroupbugs.net/view.php?id=398.  Also ensure
+     that no out-of-bounds writes occur.  */
   {
     int errs[] = { EACCES, 0, -3, };
     int j;
+
+    buf[sizeof buf - 1] = '\0';
     for (j = 0; j < SIZEOF (errs); j++)
       {
         int err = errs[j];
@@ -97,10 +101,11 @@ main (void)

         strerror_r (err, buf2, sizeof buf2);
         len = strlen (buf2);
+        ASSERT (len < sizeof buf);

         for (i = 0; i <= len; i++)
           {
-            strcpy (buf, "BADFACE");
+            memset (buf, '^', sizeof buf - 1);
             errno = 0;
             ret = strerror_r (err, buf, i);
             ASSERT (errno == 0);
@@ -108,13 +113,12 @@ main (void)
               ASSERT (ret == ERANGE || ret == EINVAL);
             else
               ASSERT (ret == ERANGE);
-            if (i == 0)
-              ASSERT (strcmp (buf, "BADFACE") == 0);
-            else
+            if (i)
               {
                 ASSERT (strncmp (buf, buf2, i - 1) == 0);
                 ASSERT (buf[i - 1] == '\0');
               }
+            ASSERT (strspn (buf + i, "^") == sizeof buf - 1 - i);
           }

         strcpy (buf, "BADFACE");
-- 
1.7.4.4