Re: [PATCH] use lockfile.c routines in git_commit_set_multivar()
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 26, 2007, 18:31 UTC
- Message-ID
- <Pine.LNX.4.64.0707261926590.14781@racer.site>
- In-Reply-To
- <11854689283208-git-send-email-bradford.carl.smith@gmail.com>
Hi,
I like the general idea. Thanks.
On Thu, 26 Jul 2007, Bradford C. Smith wrote:
Show 16 quoted lines
> + /* 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