Re: [ANNOUNCE] CGit v0.1-pre

[Linus Torvalds <torvalds@osdl.org>]

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:

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