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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]