Re: [PATCH 11/11] walker.c: use ref transaction for ref updates
From: Michael Haggerty <hidden>
Date: 2016-06-15 23:00:44
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@alum.mit.edu http://softwareswirl.blogspot.com/