[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] strerror_r: enforce POSIX recommendations
|
From: |
Eric Blake |
|
Subject: |
[PATCH] strerror_r: enforce POSIX recommendations |
|
Date: |
Fri, 20 May 2011 17:12:11 -0600 |
* 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.
---
This now passes on glibc and FreeBSD (with just the tests change,
those two platforms both failed). I'm still in the middle of
testing Solaris, Cygwin, mingw, AIX, and HP-UX before pushing this
(or a slightly modified version).
This justifies why I made the earlier ERANGE failure for AIX.
ChangeLog | 7 ++
doc/posix-functions/strerror_r.texi | 18 ++++--
lib/strerror_r.c | 116 ++++++++++++----------------------
tests/test-strerror_r.c | 88 ++++++++++++++++-----------
4 files changed, 113 insertions(+), 116 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index bda43a6..8d8d432 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2011-05-20 Eric Blake <address@hidden>
+ 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.
+
strerror_r: simplify AIX code.
* lib/strerror_r.c (strerror_r): Filter out buflen of 1 up front.
diff --git a/doc/posix-functions/strerror_r.texi
b/doc/posix-functions/strerror_r.texi
index 9d6639e..d21661a 100644
--- a/doc/posix-functions/strerror_r.texi
+++ b/doc/posix-functions/strerror_r.texi
@@ -45,15 +45,21 @@ strerror_r
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.
address@hidden
+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 2144fc6..7148c56 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -90,6 +90,30 @@ gl_lock_define_initialized(static, strerror_lock)
#endif
+/* Copy as much of MSG into BUF as possible. 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)
@@ -411,19 +435,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
@@ -436,9 +448,16 @@ strerror_r (int errnum, char *buf, size_t buflen)
{
extern int __xpg_strerror_r (int errnum, char *buf, size_t buflen);
+ *buf = '\0';
ret = __xpg_strerror_r (errnum, buf, buflen);
if (ret < 0)
ret = errno;
+ if (!*buf)
+ {
+ /* GNU strerror_r always returns a thread-safe untruncated
+ string; copy that into our buf. */
+ safe_copy (buf, buflen, strerror_r (errnum, buf, buflen));
+ }
}
#elif USE_SYSTEM_STRERROR_R
@@ -455,14 +474,7 @@ strerror_r (int errnum, char *buf, size_t buflen)
{
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);
@@ -481,14 +493,7 @@ strerror_r (int errnum, char *buf, size_t buflen)
stackbuf[0] = '\0'; /* in case strerror_r does nothing */
strerror_r (errnum, stackbuf, sizeof (stackbuf));
- len = strlen (stackbuf);
- if (len < buflen)
- {
- memcpy (buf, stackbuf, len + 1);
- ret = 0;
- }
- else
- ret = ERANGE;
+ ret = safe_copy (buf, buflen, stackbuf);
}
else
{
@@ -507,19 +512,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 */
@@ -555,17 +548,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);
@@ -585,17 +568,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;
@@ -613,17 +586,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);
@@ -632,6 +595,9 @@ strerror_r (int errnum, char *buf, size_t buflen)
#endif
+ if (ret == EINVAL && !*buf)
+ snprintf (buf, "Unknown error %d", errnum);
+
errno = saved_errno;
return ret;
}
diff --git a/tests/test-strerror_r.c b/tests/test-strerror_r.c
index 4828767..7bbd1e3 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);
}
return 0;
--
1.7.4.4
- [PATCH] strerror_r: enforce POSIX recommendations,
Eric Blake <=