From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Carlos O'Donell" Subject: Re: [PATCH] Gracefully handle incompatible locale data Date: Tue, 13 Oct 2015 09:30:50 -0400 Message-ID: <561D078A.5080307@redhat.com> References: <876132lbic.fsf@gnu.org> <20150922191804.GA13637@domone> <877fnijgin.fsf@gnu.org> <20150922215022.GA27201@domone> <8737y4hkrz.fsf@gnu.org> <20150924082755.GA4767@domone> <87h9mjeqyy.fsf@gnu.org> <5605BA8D.40907@redhat.com> <87h9mh5vgn.fsf@gnu.org> <5609A8E9.7050201@redhat.com> <87io6t1wbu.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org In-Reply-To: <87io6t1wbu.fsf@gnu.org> To: =?UTF-8?Q?Ludovic_Court=c3=a8s?= Cc: =?UTF-8?B?T25kxZllaiBCw61sa2E=?= , libc-alpha@sourceware.org, guix-devel@gnu.org, Roland McGrath List-Id: guix-devel.gnu.org On 09/29/2015 04:08 AM, Ludovic Courtès wrote: > "Carlos O'Donell" 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. Perfect. >> 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.) OK. > 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? If the locale-archive is upgraded to the new format with LC_COLLATE changed what happens when you run localedef --list-archive? Does it list zero locales and exit with an exit code of zero? I would hope that it prints something about the broken locale, because after the removal of the assertion you won't know anything is wrong with the archive? >> 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? It isn't introducing anything new, but you are removing an assert and we need to make sure that the intent of the design remains: Warn the user something is wrong with the locale data. I see two cases: - What does setlocale() return? - Verified. You say it returns EINVAL for the affected locale and that's perfect. - What does localedef --list-archive return? - The new LC_COLLATE format will make it's way into the binary locale archive and that means glibc can't read the locale-archive? Does it fail? exit code? If we cover those two cases with some kind of error message then I think we're done. Cheers, Carlos.