Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Sun, 25 Feb 2018 13:19:04 -0500
From: Rich Felker <dalias@...ifal.cx>
To: Bruno Haible <bruno@...sp.org>
Cc: Assaf Gordon <assafgordon@...il.com>, bug-gnulib@....org,
	musl@...ts.openwall.com
Subject: Re: localename: add support for musl libc

On Sun, Feb 25, 2018 at 11:17:08AM +0100, Bruno Haible wrote:
> Hi Assaf,
> 
> > > +#  elif defined __linux__ && HAVE_LANGINFO_H && defined NL_LOCALE_NAME
> > > +        /* musl libc */
> > 
> > A tiny comment about the comment :)
> > 
> > You wrote "musl libc", but what the "elif defined ..." is something like
> > "linux but not glibc, with langinfo.h" - which could (in theory) be
> > something other than musl-libc.
> 
> Yes, that's it. The refusal of the musl people to define a symbol such
> as __MUSL__ [1] makes it hard to write future-proof code. If someone else

The existence of it would not help futureproof and would promote
writing of non-futureproof code by hardcoding specific assumptions
about a specific version of musl rather than configure-time or
preprocessor-time detection of features.

> creates a platform that shares the same superficial characteristics
> (runs on Linux, has <langinfo.h> and NL_LOCALE_NAME) but behaves
> differently, we will accidentally run into the code intended for musl
> on that platform. Whereas the fallback code (return "" in this case)
> would be safer: it would make the unit test fail, but it would not
> lead to a compilation error or to a code dump.
> 
> And if that platform does not have an identifiying macro either, we
> really got a problem how to distinguish the two.

The comment /* musl */ above is wrong and should not have been added.
Really use of NL_LOCALE_NAME should always be preferred if it's
available, since it's a clean public interface for the functionality
desired rather than a hack poking at implementation internals. But if
you really like poking at internals for other implementations, it also
works to leave it as the fallback case after the hardcoded list of
assumptions about particular known platforms. It should just be called
something more reasonable like /* otherwise, use public NL_LOCALE_NAME
interface if the system has it */ instead of /* musl */.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.