Wed Feb 9 22:43:00 GMT 2011
On Feb 9 14:09, Eric Blake wrote:
> On 02/06/2011 02:52 AM, Corinna Vinschen wrote:
> > First of all, the existing implementation follows glibc's lead, so
> > it should stick to the API but the behavior should be fixed.
> > Other than that, the Linux approach would work best. Provide both
> > interfaces and use test macros. Fortunately they are documented in the
> > man page:
> > The XSI-compliant version of strerror_r() is provided if:
> > (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE
> > Otherwise, the GNU-specific version is provided.
> > Ideally we don't define _GNU_SOURCE by default, so this should result in
> > getting the POSIX version by default.
> Well, for backwards compatibility (as well as glibc compatibility), I
> thought it better we still get the _GNU_SOURCE version by default; but I
> can tweak things if you really want me to default to POSIX when no
> feature test macros are present. At any rate, here's the newlib side of
> the patch (cygwin will need to be patched separately).
My POV is, Linux compatibility is a nice feature, but if glibc and POSIX
disagree, then POSIX should have precedent. It's the standard. Anyway,
I'm not adamant about it, and backward-compatibility is certainly a good
point as well.
> --- a/newlib/libc/include/string.h
> +++ b/newlib/libc/include/string.h
> @@ -66,7 +66,22 @@ char *_EXFUN(strdup,(const char *));
> char *_EXFUN(_strdup_r,(struct _reent *, const char *));
> char *_EXFUN(strndup,(const char *, size_t));
> char *_EXFUN(_strndup_r,(struct _reent *, const char *, size_t));
> -char *_EXFUN(strerror_r,(int, char *, size_t));
> +/* There are two common strerror_r variants. If you request
> + _GNU_SOURCE, you get the GNU version; otherwise if you request
> + standards compliance, you get the POSIX version; otherwise (for
> + backwards compatibility) you get the GNU version. POSIX requires
> + that #undef strerror_r will still let you invoke the underlying
> + function, but we only support that with gcc. */
> +#if (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE
> +# ifdef __GNUC__
> +char *_EXFUN(strerror_r,(int, char *, size_t)) __asm__ ("__xpg_strerror_r");
Erm... shouldn't the return value be "int" rather than "char *" in this
> @@ -49,5 +64,6 @@ _DEFUN (strerror_r, (errnum, buffer, n),
> char *error;
> error = strerror (errnum);
> - return strncpy (buffer, (const char *)error, n);
> + strncpy (buffer, error, n);
> + return strlen (error) >= n ? error : buffer;
This looks wrong to me. Glibc guarantees that the copied result in
buffer is NUL-terminated, and I'm missing the "Unknown error XXX"
message. Wern't these problems the main reason for this patch? So,
shouldn't that be rather something like this?
if (strlen (error) >= n)
snprintf (buffer, n, "Unknown error %d", errnum);
else if (n > 0)
buffer = '\0';
strncat (buffer, error, n - 1);
> +_DEFUN (__xpg_strerror_r, (errnum, buffer, n),
> + int errnum _AND
> + char *buffer _AND
> + size_t n)
> + char *error;
> + error = strerror (errnum);
> + strncpy (buffer, error, n);
Same here. POSIX does not require that the string in buffer is
NUL-terminated, that's what the ERANGE is for, but the POSIX man page
also contains words about unknown error codes as a [CX] type extension:
"If the value of errnum is a valid error number, the message string
shall indicate what error occurred; otherwise, if these functions
complete successfully, the message string shall indicate that an
unknown error occurred."
I think it makes sense to implement this as well.
Cygwin Project Co-Leader
More information about the Newlib