Re: [musl] *strerror_r() bug in musl

[Rich Felker <dalias@libc.org>]

On Wed, Apr 13, 2022 at 02:16:52PM -0700, Gary E. Miller wrote:
> Yo Rich!
> 
> On Wed, 13 Apr 2022 16:38:35 -0400
> Rich Felker <dalias@libc.org> wrote:
> 
> > On Wed, Apr 13, 2022 at 10:36:51AM -0700, Gary E. Miller wrote:
> > > Yo Rich!
> > > 
> > > On Wed, 13 Apr 2022 10:05:33 -0400
> > > Rich Felker <dalias@libc.org> wrote:
> > >   
> > > > > > When _GNU_SOURCE is defined with glibc, then strerror_r()
> > > > > > returns a char *.    
> > > > > 
> > > > > I have met this in multiple places the last decade. The usual
> > > > > way to fix it is to also check for GNU libc in addition to
> > > > > _GNU_SOURCE.
> > > > > 
> > > > > #if defined (__GLIBC__) && defined (_GNU_SOURCE)
> > > > > 	/* non-standard GLIBC exception */
> > > > > #else
> > > > > 	/* standard behavior for everything else */
> > > > > #endif    
> > > > 
> > > > That, or probe for the signature with a configure-style check and
> > > > use the result of that, as in
> > > > 
> > > > #ifdef HAVE_GNU_STRERROR_R
> > > > // handle the GNU version
> > > > #else
> > > > // code written to the standard
> > > > #endif  
> > > 
> > > gpsd runs on a huge variety of hardware and software.  We used to
> > > have rats nests of #ifdef's as suggested above.  But that only
> > > works when your library code actually follows your documentation,
> > > and our dev actually read and understood your documentation.
> > > 
> > > Since you doc fails to mention this "quirk", it is not possible to
> > > forsee this issue before debugging the rare crash.  
> > 
> > Our docs say we aim to conform to ISO C and POSIX. The alternate glibc
> > strerror_r does not conform to POSIX and therefore we don't do it.
> 
> The musl docs also say you conform to FNU_SOURCE.

No it does not, and I'm not even sure what "conform to" would mean
here. The Conformance section in the Introduction covers what musl
attempts to conform to, The Library Interfaces section (where the
current manual ends) reiterates that:

"For all interfaces provided by musl that are specified by standards
to which musl aims for conformance, the relevant standards documents
are the official documentation."

The manual does say that _GNU_SOURCE exposes additional extension
interfaces. Not that it works like in glibc and changes the behavior
of standard interfaces. You read that into it. I agree reading that
into it is an easy misreading and that's why I want to make it more
clear.

> Two incompatible statements.
> 
> > This isn't musl being weird, it's glibc being weird.
> 
> Agreed.  musl is insufficiently documented, and glibc is seird.  Although
> to be fair, they invented strerror_r() first, and POSIX munged the copy.
> 
> > I agree it would
> > be helpful to highlight this difference though. We have material on
> > the wiki covering a bunch of differences from glibc, but somehow this
> > was overlooked:
> > https://wiki.musl-libc.org/functional-differences-from-glibc.html
> 
> Since it fails to mention strerror() or strerror_r(), it will never how
> up as a result of an internet search.  Easy to fix.   For you, not me.
> 
> May I suggest a more obvious place as well:
> 
> https://musl.libc.org/doc/1.1.24/manual.html
> 
> It currently says:
> 
>     _GNU_SOURCE (or _ALL_SOURCE)
> 
>     Adds everything above, plus interfaces modeled after GNU libc
>     extensions and interfaces for making use of Linux-specific features.
> 
> Change that to add:
> 
>     Except wher the GNU extensions conflict with POSIX.

Something like that. I would say that we should just be explicit that
this is about exposing additional interfaces only and does not change
the behavior of any standard interface. It's not an exception to
what's written before it. The statement before it is already accurate.
So, at the end of the bulleted list, something like:

"As interpreted by musl, feature test macros only control what
interfaces are exposed. They do not alter the behavior of any function
or change the definition of any type. In particular, `_GNU_SOURCE`
does not cause the signatures or behaviors of functions to change
where GNU libc deviated from the requirements of the standards."

> And link to the wiki on glibc
> 
> > In general, none of these affect software which is not making
> > non-portable glibc-specific assumptions.
> 
> Afer reading your doc, I thought that was the case for gpsd.
> 
> The two fixes above should be a big improvement.
> 
> > > OBTW: did I mention musl does not appear to have any #defines to
> > > specify its current version?  Or even that it is musl?  Or did I
> > > miss something else in the doc?  
> > 
> > No, that's intentional. The macros that tell you what to expect are
> > _POSIX_VERSION and others from unistd.h. Attempting to hard-code
> > asssumptions about musl is explicitly unsupported usage. You have to
> > either detect or just assume standard behavior. It's covered in the
> > FAQ:
> 
> And yet, I'm supposed to check the GNU feature macros?  So their defines
> are good?  But musl not having the equivalent is good?

If you're using __GLIBC__ to work around an intentional glibc
nonconformance issue, that's reasonable usage of it and part of the
way they intend for you to be able to use it.

> Get your story straight please.

I don't see where it's inconsistent.

- Using standard macros provided by the implementation that describe
  interfaces available: good.

- Providing macros that identify an implementation by name and version
  and expecting applications to hard-code knowledge about that
  implementation: bad.

- Doing the best you can do with what glibc gave you: okay.

Rich