Re: [PATCH 11/11] walker.c: use ref transaction for ref updates
From: Ronnie Sahlberg <hidden>
Date: 2016-06-15 23:00:45
I have updated the commit message with some text why I do not think this change is critical for this case. I will resend v2 of the patch series in a little while. On Sat, Apr 19, 2014 at 12:48 PM, Michael Haggerty [off-list ref] wrote:
On 04/17/2014 09:46 PM, Ronnie Sahlberg wrote:quoted
Switch to using ref transactions in walker_fetch(). As part of the refactoring to use ref transactions we also fix a potential memory leak where in the original code if write_ref_sha1() would fail we would end up returning from the function without free()ing the msg string.I don't have time to review this last patch this evening, but one thing struck me. It seems like the old code went to extra effort to lock all the write_ref references early in the function, whereas your modified version doesn't lock them until later. Have you verified that you are not opening a possible race condition? If so, your commit message should justify that it isn't a problem. In other words, what does the code do between the old time of locking and the new time of locking and why doesn't it care whether the references are locked? Aside from my other comments, patches 01-10 in the series looked fine. Thanks! Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/