Re: search locale archive again after alias expansion

["Carlos O'Donell" <carlos at redhat dot com>]

On 02/27/2015 01:45 AM, Alexandre Oliva wrote:
> On Feb 26, 2015, "Carlos O'Donell" <carlos@redhat.com> wrote:
> 
>> On 02/26/2015 01:12 AM, Alexandre Oliva wrote:
>>> Here's a follow-up patch that gets us rid of all the const-casting in
>>> loc_name and *name.  This ensures we won't write to stuff that should be
>>> const by accident, and avoids unsafely dereferencing pointers to
>>> pointers.
>>>
>>> Ok to install?
>  
>> Not OK, please make the patch minimal.
>  
> Please elaborate.  The minimal patch is already in, but it addresses a
> different problem.  It fixes only the warning, by introducing yet
> another unsafe cast where we had plenty.  This patch that you reject
> intends to REMOVE the unsafe casts and solve the violations of C
> aliasing rules that they cause.

Let me talk with code :-)

diff --git a/locale/findlocale.c b/locale/findlocale.c
index 5e2639b..77469f3 100644
--- a/locale/findlocale.c
+++ b/locale/findlocale.c
@@ -105,7 +105,8 @@ _nl_find_locale (const char *locale_path, size_t locale_path_len,
 {
   int mask;
   /* Name of the locale for this category.  */
-  char *loc_name = (char *) *name;
+  const char *loc_name = *name;
+  char *loc_name_copy;
   const char *language;
   const char *modifier;
   const char *territory;
@@ -161,8 +162,7 @@ _nl_find_locale (const char *locale_path, size_t locale_path_len,
       loc_name = (char *) _nl_expand_alias (*name);
       if (loc_name != NULL)
        {
-         data = _nl_load_locale_from_archive (category,
-                                              (const char **) &loc_name);
+         data = _nl_load_locale_from_archive (category, &loc_name);
          if (__builtin_expect (data != NULL, 1))
            return data;
        }
@@ -182,7 +182,7 @@ _nl_find_locale (const char *locale_path, size_t locale_path_len,
     loc_name = (char *) *name;
 
   /* Make a writable copy of the locale name.  */
-  loc_name = strdupa (loc_name);
+  loc_name_copy = strdupa (loc_name);
 
   /* LOCALE can consist of up to four recognized parts for the XPG syntax:
 
@@ -197,7 +197,7 @@ _nl_find_locale (const char *locale_path, size_t locale_path_len,
                (3) territory
                (4) modifier
    */
-  mask = _nl_explode_name (loc_name, &language, &modifier, &territory,
+  mask = _nl_explode_name (loc_name_copy, &language, &modifier, &territory,
                           &codeset, &normalized_codeset);
   if (mask == -1)
     /* Memory allocate problem.  */
---

Is that minimal? Does that solve the casting issue?

I want to reduce the surface area of the changes to support making
backports easier. I know nothing will ever making it work perfectly,
but less lines to merge by hand is better.

Cheers,
Carlos.