guard bug for strerror_r

guard bug for strerror_r

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

hi,
me again.

Digging a bit deeper in strerror_r I think the #if condition that
triggers the declaration of the POSIX version is wrong. 

The linux man page says

       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.

but the musl code has

#if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
 || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) \
 || defined(_BSD_SOURCE)
...
int strerror_r (int, char *, size_t);
...
#endif

So if __GNU_SOURCE is set, glibc chooses their own interface and
musl choose the POSIX interface.

Why is musl touching __GNU_SOURCE at all?

Jens


-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: guard bug for strerror_r

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-02-08 17:48:18 +0100]:
> The linux man page says
> 
>        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.
> 
> but the musl code has
> 
> #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
>  || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) \
>  || defined(_BSD_SOURCE)
> ...
> int strerror_r (int, char *, size_t);
> ...
> #endif
> 
> So if __GNU_SOURCE is set, glibc chooses their own interface and
> musl choose the POSIX interface.
> 
> Why is musl touching __GNU_SOURCE at all?
> 

musl provides the posix api when requested

musl provides many gnu specific apis when _GNU_SOURCE is set

but when posix and gnu collides it's always the posix api,
musl never provides broken gnu apis

at least this was the policy so far

Re: guard bug for strerror_r

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

Am Freitag, den 08.02.2013, 17:55 +0100 schrieb Szabolcs Nagy:
> musl provides the posix api when requested
> 
> musl provides many gnu specific apis when _GNU_SOURCE is set
> 
> but when posix and gnu collides it's always the posix api,

definitively a good strategy, but which reaches its limits, here.

> musl never provides broken gnu apis
> 
> at least this was the policy so far

__GNU_SOURCE is defined by the gnu platform to specify the
availability of their extensions. Unfortunately they don't have a
finer grained tool to distinguish different types of extensions they
provide. (BTW the same holds for gcc, that you only can tune with
version numbers.)

If I, as a user, define __GNU_SOURCE I expect to have the gnu
extension, if I then use strerror_r I expect to have their interface,
since this is documented like this. At least as it is now, I don't
think I have any means to distinguish the two platforms and to know
which version of strerror_r I would receive.

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: guard bug for strerror_r

[Isaac Dunham]

On Fri, 08 Feb 2013 18:30:00 +0100
Jens Gustedt <jens.gustedt@inria.fr> wrote:

> 
> __GNU_SOURCE is defined by the gnu platform to specify the
> availability of their extensions. Unfortunately they don't have a
> finer grained tool to distinguish different types of extensions they
> provide. (BTW the same holds for gcc, that you only can tune with
> version numbers.)
> 
> If I, as a user, define __GNU_SOURCE I expect to have the gnu
> extension, if I then use strerror_r I expect to have their interface,
> since this is documented like this. At least as it is now, I don't
> think I have any means to distinguish the two platforms and to know
> which version of strerror_r I would receive.

#ifdef __linux
#include <features.h> /* this is partly for this purpose */
/*including <unistd.h> would also work, and is more universal
* it's actually necessary with dietlibc, IIRC */
#if defined(__GLIBC__) && defined(__USE_GNU)
..

-- 
Isaac Dunham <idunham@lavabit.com>

Re: guard bug for strerror_r

[Rich Felker]

On Fri, Feb 08, 2013 at 10:01:25AM -0800, Isaac Dunham wrote:
> On Fri, 08 Feb 2013 18:30:00 +0100
> Jens Gustedt <jens.gustedt@inria.fr> wrote:
> 
> > 
> > __GNU_SOURCE is defined by the gnu platform to specify the
> > availability of their extensions. Unfortunately they don't have a
> > finer grained tool to distinguish different types of extensions they
> > provide. (BTW the same holds for gcc, that you only can tune with
> > version numbers.)
> > 
> > If I, as a user, define __GNU_SOURCE I expect to have the gnu
> > extension, if I then use strerror_r I expect to have their interface,
> > since this is documented like this. At least as it is now, I don't
> > think I have any means to distinguish the two platforms and to know
> > which version of strerror_r I would receive.
> 
> #ifdef __linux
> #include <features.h> /* this is partly for this purpose */

Use <stdlib.h> or similar -- it will include <features.h>, but it's
standard, so it won't break on systems that don't have it.

> /*including <unistd.h> would also work, and is more universal
> * it's actually necessary with dietlibc, IIRC */
> #if defined(__GLIBC__) && defined(__USE_GNU)
> ...

I would simply avoid _ever_ using strerror_r on GNU systems. On any
modern GNU or POSIX 2008 conforming system, you have the vastly
superior strerror_l function. It does not require you to provide a
buffer, and it's thread-safe (the buffer returned is either immutable
static or thread-local). The logic I'd recommend is:

#if _POSIX_VERSION >= 200809L || defined(__GLIBC__)
/* use strerror_l */
#else
/* use strerror_r and assume POSIX version of it */
#endif

Rich

Re: guard bug for strerror_r

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

Hello

Am Freitag, den 08.02.2013, 13:59 -0500 schrieb Rich Felker:
> > #if defined(__GLIBC__) && defined(__USE_GNU)

the __GLIBC__ thing seems to work for my case, now. strerror_r seems
to be the only interface where they use the same name with a different
interface, so I'll bug something around that.

> I would simply avoid _ever_ using strerror_r on GNU systems. On any
> modern GNU or POSIX 2008 conforming system, you have the vastly
> superior strerror_l function. It does not require you to provide a
> buffer, and it's thread-safe (the buffer returned is either immutable
> static or thread-local). The logic I'd recommend is:
> 
> #if _POSIX_VERSION >= 200809L || defined(__GLIBC__)
> /* use strerror_l */
> #else
> /* use strerror_r and assume POSIX version of it */
> #endif

Hm, I have a relatively recent system (ubuntu) and there is no trace
of strerror_l in the documentation, even in their "POSIX" man page.

And I actually use this in P99 to provide the C11 Annex K interface
strerror_s, which is much closer to strerror_r than strerror_l. Well,
we now have

strerror    C, POSIX
strerror_r  POSIX
strerror_l  POSIX
strerror_s  C

superbe. Again a case where a liaison between the C committee and the
POSIX could have helped.

Jens


-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: guard bug for strerror_r

[Rich Felker]

On Fri, Feb 08, 2013 at 08:53:51PM +0100, Jens Gustedt wrote:
> Hello
> 
> Am Freitag, den 08.02.2013, 13:59 -0500 schrieb Rich Felker:
> > > #if defined(__GLIBC__) && defined(__USE_GNU)
> 
> the __GLIBC__ thing seems to work for my case, now. strerror_r seems
> to be the only interface where they use the same name with a different
> interface, so I'll bug something around that.

Yes, that and basename are the only interfaces I'm aware of with
incompatible GNU versions. I actually opened a related bug tracker
issue:

http://sourceware.org/bugzilla/show_bug.cgi?id=15124

> > I would simply avoid _ever_ using strerror_r on GNU systems. On any
> > modern GNU or POSIX 2008 conforming system, you have the vastly
> > superior strerror_l function. It does not require you to provide a
> > buffer, and it's thread-safe (the buffer returned is either immutable
> > static or thread-local). The logic I'd recommend is:
> > 
> > #if _POSIX_VERSION >= 200809L || defined(__GLIBC__)
> > /* use strerror_l */
> > #else
> > /* use strerror_r and assume POSIX version of it */
> > #endif
> 
> Hm, I have a relatively recent system (ubuntu) and there is no trace
> of strerror_l in the documentation, even in their "POSIX" man page.

It's part of the "xlocale" set of interfaces that seem to have
originated with glibc or maybe Sun, and later made it into POSIX 2008.
It allows you to pass a locale_t argument to get the results in a
non-default locale, but you can instead just request the current
locale.

Honestly I don't know why POSIX didn't just require strerror itself to
be thread-safe, but at the time it was probably an "unacceptable
burden" to at least one implementation and then the issue was never
revisited because strerror_r already existed.

> And I actually use this in P99 to provide the C11 Annex K interface
> strerror_s, which is much closer to strerror_r than strerror_l. Well,
> we now have

Are you sure this is the best interface to provide to applications?
I'd instead provide a thread-safe interface with an interface
identical to strerror, since that's by far the easiest to use.

If on the other hand you want to provide Annex K in principle, that's
another matter, but there's a fairly large portion of it that can't be
provided portably on top of an existing libc (mainly the scanf or
printf stuff, I think).

> strerror    C, POSIX
> strerror_r  POSIX
> strerror_l  POSIX
> strerror_s  C
> 
> superbe. Again a case where a liaison between the C committee and the
> POSIX could have helped.

The problem seems to be just that C didn't have threads (and thus no
need for strerror_r or strerror_l) until C11.

Rich

Re: guard bug for strerror_r

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]

Am Freitag, den 08.02.2013, 15:01 -0500 schrieb Rich Felker:
> It's part of the "xlocale" set of interfaces that seem to have
> originated with glibc or maybe Sun, and later made it into POSIX 2008.
> It allows you to pass a locale_t argument to get the results in a
> non-default locale, but you can instead just request the current
> locale.

really bizarre that they didn't estimate it necessary to document it

> > And I actually use this in P99 to provide the C11 Annex K interface
> > strerror_s, which is much closer to strerror_r than strerror_l. Well,
> > we now have
> 
> Are you sure this is the best interface to provide to applications?

the best I don't know, but it is now a standard and provides
runtime-constraint handlers, and basic function parameter checking,
which are nice features. (Just forget about the official title "Bounds
checking interfaces".)

> I'd instead provide a thread-safe interface with an interface
> identical to strerror, since that's by far the easiest to use.
> 
> If on the other hand you want to provide Annex K in principle,

I do

> that's
> another matter, but there's a fairly large portion of it that can't be
> provided portably on top of an existing libc (mainly the scanf or
> printf stuff, I think).

yes, these don't work and since P99 isn't a library but only wrappers
I can't provide that. But for the most of it the wrappers are quite
simple and provided by P99.

> > strerror    C, POSIX
> > strerror_r  POSIX
> > strerror_l  POSIX
> > strerror_s  C
> > 
> > superbe. Again a case where a liaison between the C committee and the
> > POSIX could have helped.
> 
> The problem seems to be just that C didn't have threads (and thus no
> need for strerror_r or strerror_l) until C11.

But now they have threads, and so at the same moment of introducing
them they could have equipped them with a semantic (POSIX comes into
mind :) and used other interfaces from POSIX that already exist.

And, also I think that even before the _r version would have been a
good thing. I think the _r stands for reentrant, and for a function
that you would typically like to use in error or signal handlers, this
is a nice feature.

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: guard bug for strerror_r

[Rich Felker]

On Fri, Feb 08, 2013 at 09:31:11PM +0100, Jens Gustedt wrote:
> And, also I think that even before the _r version would have been a
> good thing. I think the _r stands for reentrant, and for a function
> that you would typically like to use in error or signal handlers, this
> is a nice feature.

Yes, but unfortunately the _r's and use of the word "reentrant" in
issue 6 and earlier were all wrong; they used it to mean thread-safe,
not reentrant. None of the _r functions are required to be reentrant.
The term POSIX uses for reenatrant is "async-signal-safe", and
strerror_r is not async-signal-safe. This is not just a theoretical
limitation; in practice, most implementations probably fail to be
reentrant due to use of a mutex protecting access to the current
locale data.

If/when musl adds any locale state that would need synchronization, I
think our strerror should still be async-signal-safe, either using
musl's recursive mutex (which is also a reentrant mutex) or lock-free
atomic logic.

Rich

Re: guard bug for strerror_r

[Rob Landley]

On 02/08/2013 11:30:00 AM, Jens Gustedt wrote:
> Am Freitag, den 08.02.2013, 17:55 +0100 schrieb Szabolcs Nagy:
> > musl provides the posix api when requested
> >
> > musl provides many gnu specific apis when _GNU_SOURCE is set
> >
> > but when posix and gnu collides it's always the posix api,
> 
> definitively a good strategy, but which reaches its limits, here.

Packages that don't know what their limits are and never say "no" turn  
into hairballs like Mozilla, or OpenOffice, or anything the GNU/Dammit  
crowd has ever done.

> > musl never provides broken gnu apis
> >
> > at least this was the policy so far
> 
> __GNU_SOURCE is defined by the gnu platform to specify the
> availability of their extensions. Unfortunately they don't have a
> finer grained tool to distinguish different types of extensions they
> provide. (BTW the same holds for gcc, that you only can tune with
> version numbers.)
> 
> If I, as a user, define __GNU_SOURCE I expect to have the gnu
> extension, if I then use strerror_r I expect to have their interface,


Sucks to be you then.

Musl is never going to provide 100% of what glibc does. The package you  
want is called "glibc".

> since this is documented like this. At least as it is now, I don't
> think I have any means to distinguish the two platforms and to know
> which version of strerror_r I would receive.

I think everybody is expected to patch their own __MUSL__ macro into  
their local builds. A bit like the way mksquashfs was out of tree  
forever, and union mounts still are, or the way I maintain perl removal  
patches...

Although why you want the libc defined as a compiler builtin when  
that's not even the way glibc does it is beyond me. (#include  
<features.h> is even there on the mac, if I recall. That's something  
musl does have, and what you'd patch a #define __MUSL__ into.)

Rob