This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 02/09/2011 03:11 PM, Corinna Vinschen wrote: >> Well, for backwards compatibility (as well as glibc compatibility), I >> thought it better we still get the _GNU_SOURCE version by default; I stand corrected on glibc defaults: $ printf '#include <string.h>\n'|gcc -D_POSIX_C_SOURCE=200112L -E - \ | grep strerror_r extern int strerror_r (int __errnum, char *__buf, size_t __buflen) __asm__ ("" "__xpg_strerror_r") __attribute__ ((__nothrow__)) $ printf '#include <string.h>\n'|gcc -D_GNU_SOURCE=1 -E - \ | grep strerror_r extern char *strerror_r (int __errnum, char *__buf, size_t __buflen) __attribute__ ((__nothrow__)) __attribute__ ((__nonnull__ (2))); $ printf '#include <string.h>\n'|gcc -E - | grep strerror_r extern int strerror_r (int __errnum, char *__buf, size_t __buflen) __asm__ ("" "__xpg_strerror_r") __attribute__ ((__nothrow__)) So I'll fix that up. >> +#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 > case? Oops, yes. Will fix in v2. > >> @@ -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. The "Unknown error XXX" message is a cygwin artifact, not a newlib artifact. That is, cygwin should be using it's own strerror_r, since it provides its own strerror() with different behavior than newlib's. > Wern't these problems the main reason for this patch? So, > shouldn't that be rather something like this? > > if (strlen (error) >= n) > return error; > if (!*error) > snprintf (buffer, n, "Unknown error %d", errnum); No. I strongly feel that strerror and strerror_r should return the same strings (insofar as possible). However, newlib's strerror() returns "", not "Unknown error XXX", for an unknown string, for three reasons: 1. strerror_r must be thread-safe, but it calls strerror, so strerror must also be thread-safe in newlib (in general, POSIX does not require strerror to be thread-safe, but in that case, strerror_r cannot be implemented by calling strerror); newlib achieves its thread-safety by returning only constant strings (well, newlib also provides the _user_sterror hook, which must be thread-safe for all this to work) 2. newlib did not set aside any thread-local storage for computing an alternate string, and I don't want to change struct REENT to add such storage 3. newlib targets embedded systems, and dragging in snprintf() to compute an alternate string is bloat. Meanwhile, cygwin's overrides newlib's strerror() with its own version, and cygwin's version gets away with a thread-safe "Unknown error XXX", because it uses thread-local storage, and because it does not have the snprintf executable size concerns of embedded systems. That just means that this newlib patch now means that cygwin has to also provide its own sterror_r replacement, rather than relying on newlib, to match the fact that it already provides its own strerror() replacement. > else if (n > 0) > { > buffer[0] = '\0'; > strncat (buffer, error, n - 1); > } > return buffer; This truncates the message. But glibc's behavior is that even when n was too small, you get the untruncated message. How? By returning the original (thread-safe) message from strerror instead of the user's pointer. The glibc docs state merely that the return value is NUL-terminated, not that buf is also NUL-terminated (which, for n==0, is impossible). >> +_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. Back to the idea that strerror_r should give the same output as strerror, are you saying that I should add thread-local storage to REENT to provide the "unknown error" details instead of ""? Newlib's implementation, while not ideal, is at least compliant, without making it more complex to deal with thread-local computed strings. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |