search locale archive again after alias expansion

Message ID oregr8db48.fsf@livre.home (mailing list archive)
State Committed
Delegated to: Carlos O'Donell
Headers

Commit Message

Alexandre Oliva Jan. 6, 2015, 12:08 a.m. UTC
  On Dec  9, 2014, Alexandre Oliva <aoliva@redhat.com> wrote:

> On Sep 19, 2013, Alexandre Oliva <aoliva@redhat.com> wrote:
>> BZ #15969: search locale archive again after alias expansion

>> If a locale alias is defined in locale.alias but not in an archive,
>> and the referenced locale is only present in the archive, setlocale
>> will fail if given the alias name.  This is unintuitive.  This patch
>> fixes it, arranging for the locale archive to be searched again after
>> alias expansion.

>> for  ChangeLog

>> [BZ #15969]
>> * locale/findlocale.c (_nl_find_locale): Retry archive search
>> after alias expansion.
>> * NEWS: Updated.

> Ping?
> https://sourceware.org/ml/libc-alpha/2013-09/msg00566.html

Ping?  I enclose the entire patch below, for patchwork.  (thanks, Joseph)

for  ChangeLog

	[BZ #15969]
	* locale/findlocale.c (_nl_find_locale): Retry archive search
	after alias expansion.
	* NEWS: Updated.
---
 locale/findlocale.c |   19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)
  

Comments

Carlos O'Donell Feb. 20, 2015, 8:19 p.m. UTC | #1
On 01/05/2015 07:08 PM, Alexandre Oliva wrote:
>> Ping?
>> https://sourceware.org/ml/libc-alpha/2013-09/msg00566.html
> 
> Ping?  I enclose the entire patch below, for patchwork.  (thanks, Joseph)

This showed up on my query specifically because it was in patchwork :-)
 
I fully agree that this the right solution. The alias should be converted
to the real locale and then looked up again in the dataase.

A shame that we lack the ability to test this given the existing test
infrastructure, otherwise I'd ask for a test case.

> for  ChangeLog
> 
> 	[BZ #15969]
> 	* locale/findlocale.c (_nl_find_locale): Retry archive search
> 	after alias expansion.
> 	* NEWS: Updated.

OK to checkin.

> ---
>  locale/findlocale.c |   19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/locale/findlocale.c b/locale/findlocale.c
> index 22e8b53..dab155a 100644
> --- a/locale/findlocale.c
> +++ b/locale/findlocale.c
> @@ -156,15 +156,26 @@ _nl_find_locale (const char *locale_path, size_t locale_path_len,
>        if (__glibc_likely (data != NULL))
>  	return data;
>  
> +      /* Nothing in the archive with the given name.  Expanding it as
> +	 an alias and retry.  */
> +      loc_name = (char *) _nl_expand_alias (*name);
> +      if (loc_name != NULL)
> +	{
> +	  data = _nl_load_locale_from_archive (category, &loc_name);
> +	  if (__builtin_expect (data != NULL, 1))
> +	    return data;
> +	}

OK.

> +
>        /* Nothing in the archive.  Set the default path to search below.  */
>        locale_path = _nl_default_locale_path;
>        locale_path_len = sizeof _nl_default_locale_path;
>      }
> +  else
> +    /* We really have to load some data.  First see whether the name is
> +       an alias.  Please note that this makes it impossible to have "C"
> +       or "POSIX" as aliases.  */
> +    loc_name = (char *) _nl_expand_alias (*name);

OK.

>  
> -  /* We really have to load some data.  First see whether the name is
> -     an alias.  Please note that this makes it impossible to have "C"
> -     or "POSIX" as aliases.  */
> -  loc_name = (char *) _nl_expand_alias (*name);
>    if (loc_name == NULL)
>      /* It is no alias.  */
>      loc_name = (char *) *name;
> 
> 

Cheers,
Carlos.
  

Patch

diff --git a/locale/findlocale.c b/locale/findlocale.c
index 22e8b53..dab155a 100644
--- a/locale/findlocale.c
+++ b/locale/findlocale.c
@@ -156,15 +156,26 @@  _nl_find_locale (const char *locale_path, size_t locale_path_len,
       if (__glibc_likely (data != NULL))
 	return data;
 
+      /* Nothing in the archive with the given name.  Expanding it as
+	 an alias and retry.  */
+      loc_name = (char *) _nl_expand_alias (*name);
+      if (loc_name != NULL)
+	{
+	  data = _nl_load_locale_from_archive (category, &loc_name);
+	  if (__builtin_expect (data != NULL, 1))
+	    return data;
+	}
+
       /* Nothing in the archive.  Set the default path to search below.  */
       locale_path = _nl_default_locale_path;
       locale_path_len = sizeof _nl_default_locale_path;
     }
+  else
+    /* We really have to load some data.  First see whether the name is
+       an alias.  Please note that this makes it impossible to have "C"
+       or "POSIX" as aliases.  */
+    loc_name = (char *) _nl_expand_alias (*name);
 
-  /* We really have to load some data.  First see whether the name is
-     an alias.  Please note that this makes it impossible to have "C"
-     or "POSIX" as aliases.  */
-  loc_name = (char *) _nl_expand_alias (*name);
   if (loc_name == NULL)
     /* It is no alias.  */
     loc_name = (char *) *name;