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