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__  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.