On Wed, Feb 09, 2011 at 05:20:59PM -0700, Eric Blake wrote:
>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;
This should (as should the other usage in this file), just be "set_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
Can't we get rid of this now?
cgf
