From: Johannes Schindelin Date: Thu, 26 Jul 2007 18:31:14 GMT Subject: Re: [PATCH] use lockfile.c routines in git_commit_set_multivar() Message-ID: 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: > + /* 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