This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: search locale archive again after alias expansion
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Alexandre Oliva <aoliva at redhat dot com>
- Cc: Roland McGrath <roland at hack dot frob dot com>, libc-alpha at sourceware dot org
- Date: Fri, 27 Feb 2015 16:57:09 -0500
- Subject: Re: search locale archive again after alias expansion
- Authentication-results: sourceware.org; auth=none
- References: <orr4dao5h6 dot fsf at livre dot home> <20130918220004 dot B23492C09F at topped-with-meat dot com> <ory56t31yv dot fsf at livre dot home> <or8uigyac8 dot fsf at free dot home> <oregr8db48 dot fsf at livre dot home> <54E796D1 dot 40502 at redhat dot com> <oregpd19rz dot fsf at livre dot home> <54EF93B1 dot 60808 at redhat dot com> <orioenettk dot fsf at livre dot home> <54F0A592 dot 7090809 at redhat dot com> <orfv9rawye dot fsf at livre dot home> <54F0DF2F dot 9020404 at redhat dot com> <or7fv3avjk dot fsf at livre dot home>
On 02/27/2015 04:34 PM, Alexandre Oliva wrote:
> On Feb 27, 2015, "Carlos O'Donell" <carlos@redhat.com> wrote:
>
>> That is ~9 lines of changes vs. the original ~22 lines of change.
>
> Yeah, it is smaller, but it still covers a larger code area and thus
> *more* lines of context, that will make patches in this area just as
> hard, if not harder, to apply cleanly. Plus, by not moving the
> declaration down and changing the type of a variable, patches that apply
> to the now-const area and introduce further references to loc_name might
> still seem to apply with minor adjustments, even though they might
> require further adjustments to account for the type change.
That is probably the best argument for your patch and should have been
right up there with the original patch submission.
I think the key point is that your patch *appears* to be larger than
required, and that raises a red flag for me. Then I have to go digging
to determine why you arrived at your solution. You didn't make it easy
for me, the grader, to give you an A.
I see now that your original patch was 4 hunks vs my 6, covering a smaller
area of code. It also reduces the likelihood of adding back non-const
uses of loc_name during backpors (by renaming to cloc_name) which is
important. It therefore seems like the best balance of both changes.
I'm sorry for the back and forth.
Could you please checkin your original solution as-is?
Cheers,
Carlos.