fix perror POSIX compliance

[Eric Blake <eblake at redhat dot com>]

Nobody complained after my original proposal to add a strerror wrapper
and change the _user_strerror signature to achieve POSIX compliance of
perror (POSIX states that perror must not clobber strerror's buffer,
which was not possible in newlib if you provided a one-argument
_user_strerror):

http://sourceware.org/ml/newlib/2011/msg00077.html

so here goes.  This causes a link failure for any implementation that
replaces strerror but not the new _strerror_r (think cygwin), so I will
be doing a followup cygwin patch.

 newlib/ChangeLog                    |   11 ++++++
 newlib/libc/include/string.h        |    3 ++
 newlib/libc/posix/collate.c         |    3 +-
 newlib/libc/stdio/perror.c          |    3 +-
 newlib/libc/string/strerror.c       |   62
++++++++++++++++++++++++----------
 newlib/libc/string/strerror_r.c     |    8 +++--
 newlib/libc/string/u_strerr.c       |    6 ++-
 newlib/libc/string/xpg_strerror_r.c |    5 ++-
 8 files changed, 74 insertions(+), 27 deletions(-)

2011-05-23  Eric Blake  <eblake@redhat.com>

	* libc/string/strerror.c (strerror): Split body into...
	(_strerror_r): ...new reentrant function.
	* libc/string/u_strerr.c (_user_strerror): Update signature.
	* libc/include/stdio.h (_strerror_r): New prototype.
	* libc/posix/collate.c (__collate_err): Adjust callers.
	* libc/stdio/perror.c (_perror_r): Likewise.
	* libc/string/strerror_r.c (strerror_r): Likewise.
	* libc/string/xpg_strerror_r.c (__xpg_strerror_r): Likewise.

diff --git a/newlib/libc/include/string.h b/newlib/libc/include/string.h
index d83fb8a..d565e8e 100644
--- a/newlib/libc/include/string.h
+++ b/newlib/libc/include/string.h
@@ -96,6 +96,9 @@ char  *_EXFUN(strsignal, (int __signo));
 int     _EXFUN(strtosigno, (const char *__name));
 #endif

+/* Recursive version of strerror.  */
+char *	_EXFUN(_strerror_r, (struct _reent *, int, int, int *));
+
 /* These function names are used on Windows and perhaps other systems.  */
 #ifndef strcmpi
 #define strcmpi strcasecmp
diff --git a/newlib/libc/posix/collate.c b/newlib/libc/posix/collate.c
index 8af8970..7be9cd8 100644
--- a/newlib/libc/posix/collate.c
+++ b/newlib/libc/posix/collate.c
@@ -177,12 +177,13 @@ __collate_err(int ex, const char *f)
 {
 	const char *s;
 	int serrno = errno;
+	int dummy;

 	/* Be careful to change write counts if you change the strings */
 	write(STDERR_FILENO, "collate_error: ", 15);
 	write(STDERR_FILENO, f, strlen(f));
 	write(STDERR_FILENO, ": ", 2);
-	s = strerror(serrno);
+	s = _strerror_r(_REENT, serrno, 1, &dummy);
 	write(STDERR_FILENO, s, strlen(s));
 	write(STDERR_FILENO, "\n", 1);
 	exit(ex);
diff --git a/newlib/libc/stdio/perror.c b/newlib/libc/stdio/perror.c
index 5dbf332..14b4d21 100644
--- a/newlib/libc/stdio/perror.c
+++ b/newlib/libc/stdio/perror.c
@@ -73,6 +73,7 @@ _DEFUN(_perror_r, (ptr, s),
        _CONST char *s)
 {
   char *error;
+  int dummy;

   _REENT_SMALL_CHECK_INIT (ptr);
   if (s != NULL && *s != '\0')
@@ -81,7 +82,7 @@ _DEFUN(_perror_r, (ptr, s),
       fputs (": ", _stderr_r (ptr));
     }

-  if ((error = strerror (ptr->_errno)) != NULL)
+  if ((error = _strerror_r (ptr, ptr->_errno, 1, &dummy)) != NULL)
     fputs (error, _stderr_r (ptr));

   fputc ('\n', _stderr_r (ptr));
diff --git a/newlib/libc/string/strerror.c b/newlib/libc/string/strerror.c
index 61e40ab..f40b8db 100644
--- a/newlib/libc/string/strerror.c
+++ b/newlib/libc/string/strerror.c
@@ -15,6 +15,7 @@ INDEX
 ANSI_SYNOPSIS
 	#include <string.h>
 	char *strerror(int <[errnum]>);
+	char *_strerror_r(int <[errnum]>, int <[internal]>, int *<[error]>);

 TRAD_SYNOPSIS
 	#include <string.h>
@@ -288,6 +289,8 @@ Strings pipe error

 o-

+<<_strerror_r>> is a reentrant version of the above.
+
 RETURNS
 This function returns a pointer to a string.  Your application must
 not modify that string.
@@ -296,10 +299,10 @@ PORTABILITY
 ANSI C requires <<strerror>>, but does not specify the strings used
 for each error number.

-Although this implementation of <<strerror>> is reentrant, ANSI C
-declares that subsequent calls to <<strerror>> may overwrite the
-result string; therefore portable code cannot depend on the reentrancy
-of this subroutine.
+Although this implementation of <<strerror>> is reentrant (depending
+on <<_user_strerror>>), ANSI C declares that subsequent calls to
+<<strerror>> may overwrite the result string; therefore portable
+code cannot depend on the reentrancy of this subroutine.

 Although this implementation of <<strerror>> guarantees a non-null
 result with a NUL-terminator, some implementations return <<NULL>>
@@ -317,15 +320,24 @@ extensibility.  <<errno.h>> defines
<[__ELASTERROR]>, which can be
 used as a base for user-defined error values.  If the user supplies a
 routine named <<_user_strerror>>, and <[errnum]> passed to
 <<strerror>> does not match any of the supported values,
-<<_user_strerror>> is called with <[errnum]> as its argument.
-
-<<_user_strerror>> takes one argument of type <[int]>, and returns a
-character pointer.  If <[errnum]> is unknown to <<_user_strerror>>,
-<<_user_strerror>> returns <[NULL]>.  The default <<_user_strerror>>
-returns <[NULL]> for all input values.
-
-Note that <<_user_sterror>> must be thread-safe and not alter <<errno>>
-if <<strerror_r>> is to comply with POSIX.
+<<_user_strerror>> is called with three arguments.  The first is of
+type <[int]>, and is the <[errnum]> value unknown to <<strerror>>.
+The second is of type <[int]>, and matches the <[internal]> argument
+of <<_strerror_r>>; this should be zero if called from <<strerror>>
+and non-zero if called from any other function; <<_user_strerror>> can
+use this information to satisfy the POSIX rule that no other
+standardized function can overwrite a static buffer reused by
+<<strerror>>.  The third is of type <[int *]>, and matches the
+<[error]> argument of <<_strerror_r>>; if a non-zero value is stored
+into that location, then <<strerror>> will set <<errno>> to that
+value, and the XPG variant of <<strerror_r>> will return that value
+(usually <[EINVAL]>) instead of zero or <[ERANGE]>.
+<<_user_strerror>> returns a <[char *]> value; returning <[NULL]>
+implies that the user function did not choose to handle <[errnum]>.
+The default <<_user_strerror>> returns <[NULL]> for all input values.
+Note that <<_user_sterror>> must be thread-safe, and only denote errors
+via the third argument rather than modifying <<errno>>, if <<strerror>>
+and <<strerror_r>> are are to comply with POSIX.

 <<strerror>> requires no supporting OS subroutines.

@@ -337,11 +349,14 @@ QUICKREF
 #include <string.h>

 char *
-_DEFUN (strerror, (errnum),
-	int errnum)
+_DEFUN (_strerror_r, (ptr, errnum, internal, errptr),
+        struct _reent *ptr _AND
+	int errnum _AND
+        int internal _AND
+        int *errptr)
 {
   char *error;
-  extern char *_user_strerror _PARAMS ((int));
+  extern char *_user_strerror _PARAMS ((int, int, int *));

   switch (errnum)
     {
@@ -798,10 +813,21 @@ _DEFUN (strerror, (errnum),
         break;
 #endif
     default:
-      if ((error = _user_strerror (errnum)) == 0)
-	error = "";
+      {
+        if (!errptr)
+          errptr = &ptr->_errno;
+        if ((error = _user_strerror (errnum, internal, errptr)) == 0)
+          error = "";
+      }
       break;
     }

   return error;
 }
+
+char *
+_DEFUN(strerror, (int),
+       int errnum)
+{
+  return _strerror_r (_REENT, errnum, 0, NULL);
+}
diff --git a/newlib/libc/string/strerror_r.c
b/newlib/libc/string/strerror_r.c
index c2057b7..d26a412 100644
--- a/newlib/libc/string/strerror_r.c
+++ b/newlib/libc/string/strerror_r.c
@@ -43,7 +43,9 @@ PORTABILITY
 <<strerror_r>> with a <[char *]> result is a GNU extension.
 <<strerror_r>> with an <[int]> result is required by POSIX 2001.
 This function is compliant only if <<_user_strerror>> is not provided,
-or if it is thread-safe and does not modify <<errno>>.
+or if it is thread-safe and uses separate storage according to whether
+the second argument of that function is non-zero.  For more details
+on <<_user_strerror>>, see the <<strerror>> documentation.

 POSIX states that the contents of <[buf]> are unspecified on error,
 although this implementation guarantees a NUL-terminated string for
@@ -55,7 +57,7 @@ provides only an empty string (unless you provide
<<_user_strerror>>).
 POSIX also recommends that unknown <[errnum]> fail with EINVAL even
 when providing such a message, however it is not a requirement and
 this implementation will return success if <<_user_strerror>> provided
-a non-empty alternate string.
+a non-empty alternate string without assigning into its third argument.

 <<strerror_r>> requires no supporting OS subroutines.

@@ -75,7 +77,7 @@ _DEFUN (strerror_r, (errnum, buffer, n),
 	char *buffer _AND
 	size_t n)
 {
-  char *error = strerror (errnum);
+  char *error = _strerror_r (_REENT, errnum, 1, NULL);

   if (strlen (error) >= n)
     return error;
diff --git a/newlib/libc/string/u_strerr.c b/newlib/libc/string/u_strerr.c
index fa4605c..7d902fe 100644
--- a/newlib/libc/string/u_strerr.c
+++ b/newlib/libc/string/u_strerr.c
@@ -1,8 +1,10 @@
 #include <_ansi.h>

 char *
-_DEFUN(_user_strerror, (errnum),
-       int errnum)
+_DEFUN(_user_strerror, (errnum, internal, errptr),
+       int errnum _AND
+       int internal _AND
+       int *errptr)
 {
   return 0;
 }
diff --git a/newlib/libc/string/xpg_strerror_r.c
b/newlib/libc/string/xpg_strerror_r.c
index ce94bd8..e503880 100644
--- a/newlib/libc/string/xpg_strerror_r.c
+++ b/newlib/libc/string/xpg_strerror_r.c
@@ -10,10 +10,11 @@ _DEFUN (__xpg_strerror_r, (errnum, buffer, n),
 	size_t n)
 {
   char *error;
+  int result = 0;

   if (!n)
     return ERANGE;
-  error = strerror (errnum);
+  error = _strerror_r (_REENT, errnum, 1, &result);
   if (strlen (error) >= n)
     {
       memcpy (buffer, error, n - 1);
@@ -21,5 +22,5 @@ _DEFUN (__xpg_strerror_r, (errnum, buffer, n),
       return ERANGE;
     }
   strcpy (buffer, error);
-  return *error ? 0 : EINVAL;
+  return (result || *error) ? result : EINVAL;
 }
-- 
1.7.5.1

	
-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature