On 02/06/2011 02:54 AM, Corinna Vinschen wrote: >> We already provide our own strerror() (it provides a better experience >> for out-of-range values that the newlib interface), but we're currently >> using the newlib strerror_r() (in spite of its truncation flaw). >> >> How should I rework this patch? > > It would be better if we implement strerror_r locally, in two versions, > just as on Linux. I think the best approach is to implement this in > newlib first (I replied to your mail there) and then, given that we use > the newlib string.h, copy the method over to Cygwin to match our current > strerror more closely.
Here's the cygwin side of things, to match newlib's <string.h> changes.
Surprisingly, strerror_r turned out to be identical even when based on
different root strerror(), so I left that inside #if 0, but it's easy
enough to kill the #if 0 if you don't want cygwin to use any of newlib's
strerror*.
---
winsup/cygwin/ChangeLog | 9 +++
winsup/cygwin/cygwin.din | 1 +
winsup/cygwin/errno.cc | 84
+++++++++++++++++++++-----------
winsup/cygwin/include/cygwin/version.h | 3 +-
4 files changed, 68 insertions(+), 29 deletions(-)
2011-02-09 Eric Blake <ebl...@redhat.com>
* errno.cc (__xpg_strerror_r): New function.
(strerror_r): Update comments to match newlib's fixes.
(strerror): Set errno on failure.
(_sys_errlist): Cause EINVAL failure for reserved values.
* cygwin.din: Export new function.
* include/cygwin/version.h (CYGWIN_VERSION_API_MINOR): Bump.
--
Eric Blake ebl...@redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
diff --git a/winsup/cygwin/cygwin.din b/winsup/cygwin/cygwin.din
index 2e7e647..780179a 100644
--- a/winsup/cygwin/cygwin.din
+++ b/winsup/cygwin/cygwin.din
@@ -1933,6 +1933,7 @@ xdrrec_skiprecord SIGFE
__xdrrec_getrec SIGFE
__xdrrec_setnonblock SIGFE
xdrstdio_create SIGFE
+__xpg_strerror_r SIGFE
y0 NOSIGFE
y0f NOSIGFE
y1 NOSIGFE
diff --git a/winsup/cygwin/errno.cc b/winsup/cygwin/errno.cc
index a9860f4..0e9c863 100644
--- a/winsup/cygwin/errno.cc
+++ b/winsup/cygwin/errno.cc
@@ -199,9 +199,9 @@ const char *_sys_errlist[] NO_COPY_INIT =
/* EL2HLT 44 */ "Level 2 halted",
/* EDEADLK 45 */ "Resource deadlock avoided",
/* ENOLCK 46 */ "No locks available",
- "error 47",
- "error 48",
- "error 49",
+ NULL,
+ NULL,
+ NULL,
/* EBADE 50 */ "Invalid exchange",
/* EBADR 51 */ "Invalid request descriptor",
/* EXFULL 52 */ "Exchange full",
@@ -210,8 +210,8 @@ const char *_sys_errlist[] NO_COPY_INIT =
/* EBADSLT 55 */ "Invalid slot",
/* EDEADLOCK 56 */ "File locking deadlock error",
/* EBFONT 57 */ "Bad font file format",
- "error 58",
- "error 59",
+ NULL,
+ NULL,
/* ENOSTR 60 */ "Device not a stream",
/* ENODATA 61 */ "No data available",
/* ETIME 62 */ "Timer expired",
@@ -224,13 +224,13 @@ const char *_sys_errlist[] NO_COPY_INIT =
/* ESRMNT 69 */ "Srmount error",
/* ECOMM 70 */ "Communication error on send",
/* EPROTO 71 */ "Protocol error",
- "error 72",
- "error 73",
+ NULL,
+ NULL,
/* EMULTIHOP 74 */ "Multihop attempted",
/* ELBIN 75 */ "Inode is remote (not really error)",
/* EDOTDOT 76 */ "RFS specific error",
/* EBADMSG 77 */ "Bad message",
- "error 78",
+ NULL,
/* EFTYPE 79 */ "Inappropriate file type or format",
/* ENOTUNIQ 80 */ "Name not unique on network",
/* EBADFD 81 */ "File descriptor in bad state",
@@ -245,17 +245,17 @@ const char *_sys_errlist[] NO_COPY_INIT =
/* ENOTEMPTY 90 */ "Directory not empty",
/* ENAMETOOLONG 91 */ "File name too long",
/* ELOOP 92 */ "Too many levels of symbolic links",
- "error 93",
- "error 94",
+ NULL,
+ NULL,
/* EOPNOTSUPP 95 */ "Operation not supported",
/* EPFNOSUPPORT 96 */ "Protocol family not supported",
- "error 97",
- "error 98",
- "error 99",
- "error 100",
- "error 101",
- "error 102",
- "error 103",
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
/* ECONNRESET 104 */ "Connection reset by peer",
/* ENOBUFS 105 */ "No buffer space available",
/* EAFNOSUPPORT 106 */ "Address family not supported by protocol",
@@ -357,27 +357,55 @@ strerror_worker (int errnum)
return res;
}
-/* strerror: convert from errno values to error strings */
+/* strerror: convert from errno values to error strings. Newlib's
+ strerror_r returns "" for unknown values, so we override it to
+ provide a nicer thread-safe result string and set errno. */
extern "C" char *
strerror (int errnum)
{
char *errstr = strerror_worker (errnum);
if (!errstr)
- __small_sprintf (errstr = _my_tls.locals.strerror_buf, "Unknown error %u",
- (unsigned) errnum);
+ {
+ __small_sprintf (errstr = _my_tls.locals.strerror_buf, "Unknown error
%u",
+ (unsigned) errnum);
+ errno = _impure_ptr->_errno = EINVAL;
+ }
return errstr;
}
+/* Newlib's <string.h> provides declarations for two strerror_r
+ variants, according to preprocessor feature macros. It does the
+ right thing for GNU strerror_r, but its __xpg_strerror_r mishandles
+ a case of EINVAL when coupled with our strerror() override.*/
#if 0
-extern "C" int
+extern "C" char *
strerror_r (int errnum, char *buf, size_t n)
{
- char *errstr = strerror_worker (errnum);
- if (!errstr)
- return EINVAL;
- if (strlen (errstr) >= n)
- return ERANGE;
- strcpy (buf, errstr);
- return 0;
+ char *error = strerror (errnum);
+ if (strlen (error) >= n)
+ return error;
+ return strcpy (buf, error);
}
#endif
+
+extern "C" int
+__xpg_strerror_r (int errnum, char *buf, size_t n)
+{
+ if (!n)
+ return ERANGE;
+ int result = 0;
+ char *error = strerror_worker (errnum);
+ {
+ __small_sprintf (error = _my_tls.locals.strerror_buf, "Unknown error %u",
+ (unsigned) errnum);
+ result = EINVAL;
+ }
+ if (strlen (error) >= n)
+ {
+ memcpy (buf, error, n - 1);
+ buf[n - 1] = '\0';
+ return ERANGE;
+ }
+ strcpy (buf, error);
+ return result;
+}
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index c757827..7246e8e 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -399,12 +399,13 @@ details. */
233: Add TIOCGPGRP, TIOCSPGRP. Export llround, llroundf.
234: Export program_invocation_name, program_invocation_short_name.
235: Export madvise.
+ 236: Export __xpg_strerror_r.
*/
/* Note that we forgot to bump the api for ualarm, strtoll, strtoull */
#define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 235
+#define CYGWIN_VERSION_API_MINOR 236
/* There is also a compatibity version number associated with the
shared memory regions. It is incremented when incompatible
--
1.7.3.3
signature.asc
Description: OpenPGP digital signature
