Re: [PATCH] use lockfile.c routines in git_commit_set_multivar()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

I like the general idea.  Thanks.


On Thu, 26 Jul 2007, Bradford C. Smith wrote:

> +	/* fd is closed, so don't try to close it below. */
> +	fd = -1;
> +	/*
> +	 * lock is committed, so don't try to roll it back below.
> +	 * NOTE: Since lockfile.c keeps a linked list of all created
> +	 * lock_file structures, it isn't safe to free(lock).  It's
> +	 * better to just leave it hanging around.
> +	 */
> +	lock = NULL;
>  	ret = 0;
>  
>  out_free:
>  	if (0 <= fd)
>  		close(fd);
> +	if (lock)
> +		rollback_lock_file(lock);

Wouldn't it be better to put the rollback_lock_file() into the if clause 
when commit failed?

Besides, I think you can safely call rollback_lock_file(lock) on a 
committed lock_file, since the name will be set to "" by the latter, which 
is checked by the former.

But I am fine with the patch as is (have not tested it, though).

Ciao,
Dscho