Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20180225234035.GO1436@brightrain.aerifal.cx>
Date: Sun, 25 Feb 2018 18:40:35 -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: Re: localename: add support for musl libc

On Sun, Feb 25, 2018 at 10:19:06PM +0100, Bruno Haible wrote:
> Hi Rich,
> 
> > 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 ...
> 
> In a perfect world, what you say would make sense.
> 
> However, not all libc versions that define _NL_LOCALE_NAME also have
> a _NL_LOCALE_NAME that *works* [1]. It's not that I "really like poking
> at internals". It's that I want my code to actually work.

Which ones does it not work on, and what happens when it doesn't work?
If there are libcs that advertise a feature in their headers but don't
actually support it, it sounds like they're the ones that need
special-casing. (I think it's old glibc, or some mismatched version of
glibc headers and .so, in which case it's already special-cased,
right?) But probably it's just a clean failure at runtime (like
returning a null pointer) where you can try something else if it
failed.

> > The comment /* musl */ above is wrong and should not have been added.
> 
> How can you judge that a comment in gnulib code is adequate, when you
> are not familiar with the way gnulib is developed?
> 
> The comment /* musl */ says two things:
>   - If a developer makes changes to these piece of code, they should
>     test in on a system with musl libc.
>   - If a developer sees that this code is being compiled/executed on
>     a system without musl libc, they should review the #if chain, to
>     make sure no mistake was introduced in #ifs.

OK. I think it would have been clearer as:

	/* Known to work and used on musl libc. Also present
	 * but not always working on [glibc?] so we use the
	 * above case... */

If /* musl */ is fine for conveying that to the people who work on
your code, that's one thing, but I still think it's misleading to
others who come across and read it, because this is not an interface
musl invented but rather one we adopted because glibc, and maybe other
systems, already invented something that looked clean & reasonable to
solve a problem that had no other clean solution.

> Now back to my comment that you haven't addressed, regarding lack of
> __MUSL__:
> 
>   If someone else
>   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.

I disagree with the characterization of this code as "intended for
musl". It's possible that this is someone's intent, but it's not a
programming model musl aims to support. Rather I would see it as code
"intended for systems that provide the NL_LOCALE_NAME langinfo item to
query the name of the active locale". Whether musl is such a system is
version-dependent (1.1.17 or later) and easily determined via
configure- or preprocessor-time tests.

As for failure on other systems, it's testable at configure time that
NL_LOCALE_NAME is a macro accepting locale category macros as
arguments and producing an integer result, demonstrating that it won't
produce a compile error. In that case the worst thing that happens,
unless nl_langinfo is buggy, is returning an empty string or some
string that's not the locale name. (I know at one point in the past
musl erroneously returned null for undefined items, so it might be
worth checking for that too; other implementations might also have had
that bug at some point.)

>   And if that platform does not have an identifiying macro either, we
>   really got a problem how to distinguish the two.

I'm not clear what further distinguishing is needed. We've discussed
before and are open to designing an approach to advertise extended
functionality via macros defined in the standard headers -- something
like the _POSIX_* macros unistd.h defines, but for extensions. But not
just an "I am musl, go assume whatever was true in the version of musl
you looked at" macro. But in order for this to move forward there
should really be some interest in collaboration between
implementations (I think glibc would be open to it now, maybe one or
more BSDs too) and input from application programers (the consumers of
the macros) on what would be useful to them.

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.