Re: [PATCH 4/5] Log ref updates made by fetch.

[Junio C Hamano <junkio@cox.net>]

Shawn Pearce <spearce@spearce.org> writes:

> diff --git a/refs.c b/refs.c
> index 31cf276..d3ddc82 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -142,6 +142,8 @@ static int do_for_each_ref(const char *b
>  			namelen = strlen(de->d_name);
>  			if (namelen > 255)
>  				continue;
> +			if (namelen>5 && !strcmp(de->d_name+namelen-5,".lock"))
> +				continue;
>  			memcpy(path + baselen, de->d_name, namelen+1);
>  			if (stat(git_path("%s", path), &st) < 0)
>  				continue;

Now this got me worried.  Until now I did not realize that we
are clobbering refnames that ends with ".lock" if another ref
with the name without ".locK" is updated.  Because we do not
forbid a name that ends with ".lock" to be used as a refname,
this is an accident waiting to happen.

Not your fault, though.  It was like this ever since the initial
version of refs.c was accepted by Linus.

We could do one of two things: officially forbid any refname
that ends with ".lock", or fix the lockfile naming convention.

Nobody should be relying on what the actual lockfile-to-be-
renamed-to-become-the-real-file is called.  I suspect we would
want to fix refs.c::ref_lock_file_name() to use a name that
would never be used as a refname.

We could make it begin with ".", so the lock file for the master
".git/refs/heads/master" would become ".git/refs/heads/.master",
for example.  That way, we cannot clobber a valid unrelated ref
(".master" is not a valid ref name), and as an added bonus, you
do not even have to have the above hunk.

Hmm?