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 <mhagger@xxxxxxxxxxxx> wrote: > On 04/17/2014 09:46 PM, Ronnie Sahlberg wrote: >> 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@xxxxxxxxxxxx > http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html