Re: [PATCH] Fix broken sha1 locking

From: Petr Baudis <pasky@suse.cz>
Date: Sep 19, 2006, 21:23 UTC
Message-ID: <20060919212340.GC8259@pasky.or.cz>
In-Reply-To: <Pine.LNX.4.64.0609191411460.4388@g5.osdl.org>

Dear diary, on Tue, Sep 19, 2006 at 11:16:04PM CEST, I got a letter where Linus Torvalds <torvalds@osdl.org> said that...

Show 13 quoted lines

> On Tue, 19 Sep 2006, Petr Baudis wrote:
> >
> > Current git#next is totally broken wrt. cloning over HTTP, generating refs
> > at random directories. Of course it's caused by the static get_pathname()
> > buffer. lock_ref_sha1() stores return value of mkpath()'s get_pathname()
> > call, then calls lock_ref_sha1_basic() which calls git_path(ref) which
> > calls get_pathname() at that point returning pointer to the same buffer.
> 
> Hmm. This was exactly the schenario why I did commit
> e7676d2f6454c9c99e600ee2ce3c7205a9fcfb5f - allowing a couple of 
> overlapping paths
> 
> Isn't that in the "next" branch too?

Yes, and between the mkpath() and git_path() calls exactly three other get_pathname() calls happen.

Of course you could just enlarge the cache, but that will merely make the bugs even harder to spot, let alone track down. I argue that having this cache at all is harmful and will result in bugs over time as new get_pathname() calls are added in the middle of currently safe "concurrent" calls.

> Of course, that still assumes that you strdup() the result at _some_ time, 
> and can't just save it away, but lock_ref_sha1_basic() should do that.

lock_ref_sha1_basic() never strdup()s ref (at least the reference used for git_path() later).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam