From 3a3fe58b91a00ba3cafd960a2e2355c842448e47 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 18 May 2011 15:19:51 -0600 Subject: [PATCH] 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 --- ChangeLog | 9 +++ doc/posix-functions/strerror_r.texi | 18 +++-- lib/strerror_r.c | 114 +++++++++++----------------- tests/test-strerror_r.c | 88 ++++++++++++--------- 4 files changed, 118 insertions(+), 111 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4ec646385e..bc838bdacb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2011-05-24 Eric Blake + + 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 test-perror2.c: avoid warning about unused variable diff --git a/doc/posix-functions/strerror_r.texi b/doc/posix-functions/strerror_r.texi index 21e4181186..91026ef912 100644 --- a/doc/posix-functions/strerror_r.texi +++ b/doc/posix-functions/strerror_r.texi @@ -51,15 +51,21 @@ This function always fails when the third argument is less than 80 on some 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 diff --git a/lib/strerror_r.c b/lib/strerror_r.c index f0b59c717c..40ebc59814 100644 --- a/lib/strerror_r.c +++ b/lib/strerror_r.c @@ -92,6 +92,30 @@ gl_lock_define_initialized(static, strerror_lock) #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 @@ strerror_r (int errnum, char *buf, size_t buflen) if (buflen <= 1) { if (buflen) - *buf = 0; + *buf = '\0'; return ERANGE; } + *buf = '\0'; #if GNULIB_defined_ETXTBSY \ || GNULIB_defined_ESOCK \ @@ -413,19 +438,7 @@ 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); - ret = 0; - } - errno = saved_errno; - return ret; - } + return safe_copy (buf, buflen, msg); } #endif @@ -441,6 +454,13 @@ strerror_r (int errnum, char *buf, size_t buflen) 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 @@ strerror_r (int errnum, char *buf, size_t buflen) { 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 @@ strerror_r (int errnum, char *buf, size_t buflen) /* 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 @@ strerror_r (int errnum, char *buf, size_t buflen) 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 @@ strerror_r (int errnum, char *buf, size_t buflen) 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 @@ strerror_r (int errnum, char *buf, size_t buflen) 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 @@ strerror_r (int errnum, char *buf, size_t buflen) #endif + if (ret == EINVAL && !*buf) + snprintf (buf, buflen, "Unknown error %d", errnum); + errno = saved_errno; return ret; } diff --git a/tests/test-strerror_r.c b/tests/test-strerror_r.c index 9c4874fb75..1f23ba8f82 100644 --- a/tests/test-strerror_r.c +++ b/tests/test-strerror_r.c @@ -36,21 +36,24 @@ main (void) 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 @@ main (void) 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. 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; - /* Test results with a too small buffer. */ + strerror_r (err, buf2, sizeof buf2); + len = strlen (buf2); - ASSERT (strerror_r (EACCES, buf, sizeof (buf)) == 0); - { - size_t len = strlen (buf); - size_t 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'); + } + } - for (i = 0; i <= len; i++) - { 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 -- 2.30.2