From 79d4e75d8e14dee5d91f58413942fe875857d4f5 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 7 Jun 2011 20:49:04 -0600 Subject: [PATCH] strerror_r-posix: fix on MacOS 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 --- 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 9453f3bc8..6cd875732 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2011-06-08 Eric Blake + + 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 gnulib-tool: Better isolation between different gnulib-tool invocations. diff --git a/doc/posix-functions/perror.texi b/doc/posix-functions/perror.texi index ddc7c9b96..c11d2e688 100644 --- a/doc/posix-functions/perror.texi +++ b/doc/posix-functions/perror.texi @@ -15,7 +15,7 @@ OpenBSD 4.0, OSF/1 5.1, Cygwin 1.5.x, mingw. @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 6f9519a52..931ab4fdd 100644 --- a/doc/posix-functions/strerror.texi +++ b/doc/posix-functions/strerror.texi @@ -13,10 +13,11 @@ This function does not support the error values that are specified by POSIX 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 -@code{strerror(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 +@code{errno} 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 4169ca9ab..5aaa1c641 100644 --- a/doc/posix-functions/strerror_r.texi +++ b/doc/posix-functions/strerror_r.texi @@ -45,7 +45,11 @@ OpenBSD 4.0, OSF/1 5.1, NonStop Kernel, Cygwin 1.5.x. @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. +@item +This function produces a different string for @code{0} than +@code{strerror} 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 4dc0b65c1..d0dd1af98 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 d0c7be953..46b47ed8c 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 048b03c0c..03a0b1a39 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 ]], [[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 68a0b6911..89758273e 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 03637d785..3ffb12e8f 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 1f23ba8f8..956c458d7 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"); -- 2.11.0