Re: [PATCH] strerror_r: fix on newer cygwin

[Eric Blake]

On 05/19/2011 02:51 PM, Bruno Haible wrote:
>> in the process of adding
>> __xpg_sterror_r, the original strerror_r was fixed to align more closely
>> to glibc behavior.  Thus, it was changed so that strerror_r(EACCES, buf,
>> 2) no longer overwrites buf with a (truncated) message to buf, so the
>> caller instead has to manually copy the static string result over to buf.
>>
> Thanks for explaining. But that means that a binary built with Cygwin
> 1.7.5 will still fail when run on Cygwin >= 1.7.9. The complete fix
> ought to look something like this, I guess?

Indeed, and good catch.

> +++ lib/strerror_r.c  Thu May 19 22:48:05 2011
> @@ -456,10 +456,34 @@
>          ret = strerror_r (errnum, buf, buflen);
>      }
>  # elif defined __CYGWIN__
> -    /* Cygwin 1.7.8 only provides the glibc interface, is thread-safe, and
> -       always succeeds (although it may truncate). */
> -    strerror_r (errnum, buf, buflen);
> -    ret = 0;
> +    /* Cygwin <= 1.7.8 only provides the glibc interface, is thread-safe, and
> +       always succeeds (although it may truncate).  In Cygwin >= 1.7.9, 
> instead
> +       of truncating, it leaves the buffer untouched.  */

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.


> +    {
> +      char stackbuf[256];
> +
> +      if (buflen < sizeof (stackbuf))
> +        {
> +          size_t len;
> +
> +          stackbuf[0] = '\0'; /* in case strerror_r does nothing */
> +          strerror_r (errnum, stackbuf, sizeof (stackbuf));
> +          len = strlen (stackbuf);
> +          if (len < buflen)
> +            {
> +              memcpy (buf, stackbuf, len + 1);
> +              ret = 0;
> +            }
> +          else
> +            ret = ERANGE;
> +        }
> +      else
> +        {
> +          buf[0] = '\0'; /* in case strerror_r does nothing */
> +          strerror_r (errnum, buf, buflen);
> +          ret = 0;
> +        }
> +    }

Yes, that looks good, other than any comment wording tweaks you might want.

[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]

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

signature.asc
Description: OpenPGP digital signature