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

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