Re: [ANNOUNCE] CGit v0.1-pre

From: Linus Torvalds <torvalds@osdl.org>
Date: Dec 11, 2006, 18:18 UTC
Message-ID: <Pine.LNX.4.64.0612111016220.12500@woody.osdl.org>
In-Reply-To: <8c5c35580612110940p767fd360p52762a8818fbc8c1@mail.gmail.com>

On Mon, 11 Dec 2006, Lars Hjemli wrote:

> 
> Ok. Code speeks louder than words, so I'll blatantly paste the key functions:

Yeah, they're still buggy:

Show 5 quoted lines

> int cache_unlock(struct cacheitem *item)
> {
> 	close(item->fd);
> 	return (rename(fmt("%s.lock", item->name), item->name) == 0);
> }

...

Show 8 quoted lines

> 	if (!cache_exist(item)) {
> 		if (!cache_lock(item)) {
> 			sleep(1);
> 			goto top;
> 		}
> 		if (!cache_exist(item))
> 			cgit_fill_cache(item);
> 		cache_unlock(item);

What do you think happens if that last "cache_exist()" returned true?

That's right: the "cache_unlock()" will now OVERWRITE the valid cache with the (empty) lock-file that you didn't fill in.

Oops.

So you really have two different cases:

 - the "I created the file" case: rename lockfile to final name
 - the "somebody else created the file": remove the lockfile

and you can't use the same "cache_unlock()" for both of them.