Re: [PATCH] strerror_r: fix on newer cygwin

[Eric Blake]

On 05/19/2011 08:06 PM, Bruno Haible wrote:
> Eric Blake wrote:
>> More accurately:
>>
>> The change in strerror_r behavior was in 1.7.8 (which also tried to
>> introduce __xpg_strerror_r), but a bug in the headers vs. export table
>> ended up with a link failure for __xpg_strerror_r:
>> http://cygwin.com/ml/cygwin/2011-03/msg00247.html
>>
>> In <= 1.7.7, the result was always buf, and truncation occurred for both
>> in-range and out-of-range errors.
>>
>> In >= 1.7.8, the result for in-range buffers is static and buf is
>> untouched, and out-of-range errors returns buf with possible truncation.
> 
> Thanks for the info. I've updated the comments accordingly.
> 
>> Yes, that looks good, other than any comment wording tweaks you might want.
> 
> OK. Committed and pushed.

Phooey, I've done more testing, by faking the cache variable to claim
that __xpg_strerror_r is not available on cygwin 1.7.9.

It turns out that this change is insufficient - the glibc semantics of
GNU strerror_r are that if errnum was in range, buf is _always_
untouched, and the answer is _only_ available in the returned char*
pointer, even if buf was big enough.  That is, GNU strerror_r only
modifies buf in the case of EINVAL, but does silent truncation and never
fails with ERANGE.  I'll come up with a corrective patch later today.

>> [Actually, I plan on further enhancing gnulib strerror_r to work around
>> http://sourceware.org/bugzilla/show_bug.cgi?id=12782, which may cause me
>> to have to revisit this code at the same time I add in glibc
>> workarounds, but that can come after your patch]
> 
> I would find these enhancements to the strerror_r specification useful
> as well. But I would wait before changing gnulib until the issue has been
> discussed in the Austin group and glibc has followed suit. If they start
> to disagree, it's too early for gnulib to provide the modified (proposed)
> semantics.

There's two things at stake here.

One is whether strerror_r _must_ fail with ERANGE on a too-short buffer
(well, the combination of too-short buffer and out-of-range errnum can
still fail with EINVAL); right now, POSIX leaves the ERANGE error as
optional, but http://austingroupbugs.net/view.php?id=398 will require it
to be mandatory.  Although that bug report has not yet been approved, I
see no problem in implementing it now (compare to getpwd() where ERANGE
is mandatory, and consider that without ERANGE you have no other way to
tell if truncation occurred), since POSIX does not forbid it.  glibc and
cygwin __xpg_strerror_r already fails with ERANGE on a short buffer, but
there are other platforms that do silent truncation and still return 0.

The other is whether strerror_r may modify buf even when returning
ERANGE or EINVAL.  Right now, POSIX does not forbid this, and in fact,
it even recommends it in the case of EINVAL.  I took care in
http://austingroupbugs.net/view.php?id=398 to also add wording
recommending that buf is also modified in the case of ERANGE to always
be a NUL-terminated string (except for 0-size buf).  And it is this
issue that glibc is _not_ doing (hence glibc bug 12782); glibc leaves
buf untouched on error, which is less useful to the user, because too
many people use strerror* without error checking and just assume that
the result will still be usable, but not modifying buf means that a
subsequent strlen operation could read beyond array bounds and segfault.
 Meanwhile, other platforms like BSD, or cygwin's __xpg_strerror_r,
already modify buf, so they do not need wrapping.  So again, I see no
problem guaranteeing that strerror_r always modifies buf in gnulib; in
fact, implementing it now will solve this libvirt bug report:
http://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00394.html

-- 
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature