Re: [PATCH 2/2] close_lock_file(): new function in the lockfile API
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 16, 2008, 23:19 UTC
- Message-ID
- <7vtzldmk8p.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <Pine.LNX.4.64.0801161443340.31161@torch.nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> My patch does this, though I understand it may take some time to review. > > I left the lk->fd unmodified when close() failed in case the caller > would like to include it in an error message.
But that would bring us back to the same double-close issue, wouldn't it?
if (close_lock_file(lock))
die("Oops, failed to close fd %d", lock->fd);is not enough. You need to do:
if (close_lock_file(lock)) {
int fd = lock->fd;
lock->fd = -1;
die("Oops, failed to close fd %d", fd);
}to avoid atexit handler closing the lock->fd.
Worse yet, a careless caller may do:
close_lock_file(lock);
... do something that opens a new fd, perhaps for
... mmaping a packfile inrollback_lock_file(lock);
... Oops, we cannot mmap the packfile.