From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 21 Jan 2026 16:23:50 GMT
Subject: Re: [PATCH v5] lockfile: add PID file for debugging stale locks
Message-ID: <xmqqcy33vsrt.fsf@gitster.g>
In-Reply-To: <20260121071344.GA570838@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> 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.