This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: search locale archive again after alias expansion


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.

>> [BZ #15969]
>> * locale/findlocale.c (_nl_find_locale): Introduce const
>> version of loc_name and drop unsafe type casts.

> The name change from loc_name to cloc_name makes backports to release
> branches difficult as any patches applying on top of this one will
> require this patch also.

What will make backports difficult is the removal of the casts.  Those
occur at every few lines in each hunk; no context diff that touched
lines containing the const char* variable would avoid having at least
one line that currently contains a faulty cast.

We need a const char* variable to avoid the casts and potential
violation of alias safety rules (writing a char const* to a char *
lvalue is not safe), and we need a char * variable after the copy,
because we write to the string.  They can't both have the same name,
unless they were in different scopes, which would require reindenting,
making the differences even bigger.

Now, if I were to rename the non-const version, as Andreas suggested,
this would *increase*, rather than decrease, the patch conflict surface
area, because we'd have conflicts due to the dropped casts, in the area
where I renamed the variable, and more conflicts due to the use of the
renamed non-const char * variable after we copy the const string to a
writable region.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]