diff mbox

search locale archive again after alias expansion

Message ID 54F0A592.7090809@redhat.com
State New
Headers show

Commit Message

Carlos O'Donell Feb. 27, 2015, 5:12 p.m. UTC 
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 :-)

---

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.

Comments

Alexandre Oliva Feb. 27, 2015, 9:03 p.m. UTC | #1 
On Feb 27, 2015, "Carlos O'Donell" <carlos@redhat.com> wrote:

> Is that minimal? Does that solve the casting issue?

Of course not.  Plenty of casts that drop const, now superfluous, remain
in place with your patch.  The point of the patch was to remove them
all.
diff mbox

Patch

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.  */