On Thu, May 16, 2013 at 09:31:54AM -0400, Carlos O'Donell wrote:
On 05/16/2013 02:06 AM, Siddhesh Poyarekar wrote:
> Hi,
>
> A patch I had pushed into 2.16 had an error which resulted in the
> regression described in the aforementioned bug as well as the bug
> described in bz @15339. Attached patch fixes this. If it looks OK,
> I'll apply it in rawhide and f19. I have also posted it upstream:
This class of change is interesting, in that nobody can easily review
it because it was a test-driven change. The patch adjusts the code
in response to feedback from tests run by users with observed versus
expected output being adjusted.
Reviewing the code is easy, it looks right, it isn't invalid C,
and it's only three lines of change.
However, the existing implementation is so complex and subtle that
it would take me days to review this code well.
Agreed, these bits are an ugly maze.
What would help in the review of this, and the upstream review, is
to have a detailed breakdown of the analysis you did while fixing
this.
Do you have such an analysis?
I don't have a written detailed breakdown right now, but I don't mind
doing it. I'll post an analysis of this soon. Meanwhile, here's the
original patch which caused this breakage, in case that helps build
some of the context:
http://sourceware.org/ml/libc-alpha/2012-10/msg00611.html
Siddhesh