Show 31 quoted lines
> On Wed, Jan 21, 2026 at 2:15 AM Jeff King <peff@peff.net> wrote:
>> 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
>> @@ -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?
>
> Correct me if I'm wrong, but wouldn't this suggested change be
> problematic on Microsoft Windows? Specifically, if I recall correctly,
> Windows won't allow a file to be deleted if any processes still have
> it open, and this change eliminates the call to close() preceding the
> call to unlink(), so the file would still be held open when the
> attempt is made to remove it.