Re: [PATCH v5] lockfile: add PID file for debugging stale locks
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 21, 2026, 16:23 UTC
- Message-ID
- <xmqqcy33vsrt.fsf@gitster.g>
- In-Reply-To
- <20260121071344.GA570838@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 26 quoted lines
> I don't think it's wrong, but the cleanup is redundant between the "out"
> path and the others.
>
> Did you mean this:
>
> diff --git a/lockfile.c b/lockfile.c
> index 731cdd4944..e5d6ae0df6 100644
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -122,14 +122,10 @@ static struct tempfile *create_lock_pid_file(const char *pid_path, int mode)
> strbuf_addf(&content, "pid %" PRIuMAX "\n", (uintmax_t)getpid());
> if (write_in_full(fd, content.buf, content.len) < 0) {
> warning_errno(_("could not write lock pid file '%s'"), pid_path);
> - close(fd);
> - fd = -1;
> unlink(pid_path);
> goto out;
> }
>
> - close(fd);
> - fd = -1;
> pid_tempfile = register_tempfile(pid_path);
>
> out:
>
> which would just let the close after the out label handle all cases?I recall suggesting this myself in
https://lore.kernel.org/git/xmqqbjj4hnkr.fsf@gitster.g
without realizing that this would probably not work on Windows where unlink() cannot work correctly until the file descriptor is closed.