From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Johannes Sixt" <j6t@kdbg.org>,
"Torsten Bögershausen" <tboegi@web.de>,
"Ronnie Sahlberg" <sahlberg@google.com>,
"Jonathan Nieder" <jrnieder@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH 0/3] Support stdio access to lockfiles
Date: Wed, 1 Oct 2014 08:52:15 -0400 [thread overview]
Message-ID: <20141001125214.GC10772@peff.net> (raw)
In-Reply-To: <1412162089-3233-1-git-send-email-mhagger@alum.mit.edu>
On Wed, Oct 01, 2014 at 01:14:46PM +0200, Michael Haggerty wrote:
> This series applies on top of the series "Lockfile correctness and
> refactoring" (Junio's branch mh/lockfile).
>
> There are already two callers that write to lockfiles using stdio. But
> they currently need intimate knowledge of the lockfile implementation
> to work correctly; for example, they have to call fclose() themselves
> and set lk->fd to -1 to prevent the file from being closed again. This
> is awkward and error-prone.
I think it's also wrong on systems where you cannot delete an open file.
A signal or atexit handler will not be able to close the file (since it
does not know the fd), and the unlink() will fail. IIRC, either cygwin
or msysgit (or both?) is such a platform.
> Michael Haggerty (3):
> fdopen_lock_file(): access a lockfile using stdio
> dump_marks(): reimplement using fdopen_lock_file()
> commit_packed_refs(): reimplement using fdopen_lock_file()
I had a few minor comments on the first patch, but I would also be OK
with it going in as-is. The other two looked fine to me. Thanks for
working on this.
-Peff
prev parent reply other threads:[~2014-10-01 12:52 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-01 11:14 [PATCH 0/3] Support stdio access to lockfiles Michael Haggerty
2014-10-01 11:14 ` [PATCH 1/3] fdopen_lock_file(): access a lockfile using stdio Michael Haggerty
2014-10-01 12:48 ` Jeff King
2014-10-01 21:20 ` Junio C Hamano
2014-10-02 9:29 ` Torsten Bögershausen
2014-10-12 6:17 ` Michael Haggerty
2014-10-01 11:14 ` [PATCH 2/3] dump_marks(): reimplement using fdopen_lock_file() Michael Haggerty
2014-10-01 11:14 ` [PATCH 3/3] commit_packed_refs(): " Michael Haggerty
2014-10-01 12:52 ` Jeff King [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141001125214.GC10772@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=jrnieder@gmail.com \
--cc=mhagger@alum.mit.edu \
--cc=sahlberg@google.com \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).