On 05/20/2011 04:50 PM, Bruno Haible wrote:
> Hi Eric,
>
>> @@ -95,6 +95,15 @@ int
>> strerror_r (int errnum, char *buf, size_t buflen)
>> #undef strerror_r
>> {
>> + /* Filter this out now, so that rest of this replacement knows that
>> + there is room for a non-empty message and trailing NUL. */
>> + if (buflen <= 1)
>> + {
>> + if (buflen)
>> + *buf = 0;
>> + return ERANGE;
>> + }
>> +
>> #if GNULIB_defined_ETXTBSY \
>> || GNULIB_defined_ESOCK \
>> || GNULIB_defined_ENOMSG \
>
> This is odd and inconsistent.
> errnum = -3, buflen >= 2 -> return EINVAL
> errnum = -3, buflen <= 1 -> return ERANGEBut both errors are allowed by POSIX, and we have the case that glibc's __xpg_strerror_r favors EINVAL while cygwin's _xpg_strerror_r favors ERANGE. The testsuite tests for both codes, because there is no heirarchy in POSIX on which error code if more than one error code simultaneously applies. I can go with an additional restriction in the testsuite to _always_ favor ERANGE over EINVAL, but that would mean that glibc's __xpg_strerror_r would also be replaced; and our goal has typically been to avoid replacing glibc if it already complies with POSIX. > > Also it seems strange to introduce new code for all platforms, > from FreeBSD to Cygwin, with the rationale that it's a simplification > for AIX. Yes, but it _also_ simplifies my next patch, now posted, which touches glibc, cygwin, bsd, hp-ux, and solaris code; several of which all benefitted by having buflen of 1 already filtered out in advance. > > I would not have done this. Rather, since the caller does not know > in advance how long the string will be (i.e. how long the buffer needs > to be for a complete message), that ERANGE should be the "lowest priority" > error. That is, I think EINVAL should take precedence over ERANGE. I personally disagree - if I call strerror_r(-12, buf, 17) and get ERANGE, then I immediately know that "Unknown error -1" is suspect; but if I get EINVAL, then I've just printed a misleading error number, and the only way to tell if I didn't suffer from truncation is to retry with a larger buffer until I get an answer with a strlen() smaller than my buffer. That is, if I am going to _guarantee_ that all errors produce a non-empty message, I'd much rather prefer learning that my buffer was too small until I got my complete non-empty message. > snprintf: > > This function overwrites memory even when a size argument of 1 is passed on > some > platforms: > Linux libc5. Yep - early exit on buflen <= 1 is what _lets_ me use snprintf in the next patch, without having to drag in the snprintf module. > > then it is because these system functions were coded with the "let's check the > size upfront" philosophy. And now you introduce the same, broken idiom here? If you _guarantee_ a non-empty string, then there are only two lengths that cannot produce such a string: 0 and 1. And those are the two where we do the early exit, and avoid all the shenanigans of calling the system's strerror just to find out that we'd be returning an error and an empty string anyways. -- Eric Blake ebl...@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
