Re: [PATCH] Gracefully handle incompatible locale data

[Ludovic Courtès]

"Carlos O'Donell" <address@hidden> skribis:

> On 09/26/2015 06:24 AM, Ludovic Courtès wrote:
>> Furthermore, the function in question returns EINVAL in other similar
>> cases–e.g., when libc 2.22 loads LC_COLLATE data from libc 2.21.
>
> If you change this particular case to EINVAL, what does the user see
> as a result of this change?

The user-visible change is that, if incompatible or broken locale data
is found, a call like:

  setlocale (LC_ALL, "");

returns EINVAL instead of aborting.

> Do they get a non-zero exit code from `localedef --list-archive` along
> with an error written out to stderr?

‘localedef’ starts with:

  setlocale (LC_MESSAGES, "");
  setlocale (LC_CTYPE, "");

so it will no longer abort when invalid locale data is found (although
in the 2.21 → 2.22 transition, only the LC_COLLATE data format differs
anyway.)

Apart from that, ‘localedef --list-archive’ simply opens the locale
archive (typically /usr/lib/locale/locale-archive, regardless of the
‘LOCPATH’ environment variable value), so its behavior is unchanged.

Am I overlooking something?

> This is the kind of change I'm expecting. If we are removing an assertion,
> we should be replacing it with something meaningful and verifying that
> meaningful change.

Yes, agreed.

The function that is changed, ‘_nl_intern_locale_data’, has only two
callers in libc, and both check whether it returns NULL.  So it seems to
me that the code is not introducing anything new in the API contract.
WDYT?

Thank you,
Ludo’.