Subject: Re: [PATCH 11/11] walker.c: use ref transaction for ref updates

From: Michael Haggerty <[email-masked]>
Date: Sat, 19 Apr 2014 21:48:42 +0200

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
[email-masked]
http://softwareswirl.blogspot.com/