All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Johannes Sixt" <j6t@kdbg.org>,
	"Torsten Bögershausen" <tboegi@web.de>,
	"Jeff King" <peff@peff.net>,
	"Ronnie Sahlberg" <sahlberg@google.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH v6 02/39] api-lockfile: revise and expand the documentation
Date: Tue, 30 Sep 2014 15:41:55 +0200	[thread overview]
Message-ID: <542AB323.4080402@alum.mit.edu> (raw)
In-Reply-To: <xmqq8ul6869o.fsf@gitster.dls.corp.google.com>

On 09/26/2014 10:40 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> +If you need to close the file descriptor you obtained from a
>> +`hold_lock_file_*` function yourself, do so by calling
>> +`close_lock_file`. You should never call `close(2)` yourself!
> 
> This is sometimes untenable, isn't it?  A caller may want to
> freopen(3) a stream on the fd to write into it and then fclose(3);
> which would not know to call close_lock_file() but calls close(2).

You are absolutely correct. I carried this text over from the old
version, where it was just as inaccurate.

I didn't fix it because IMO the correct fix is to add a stdio-oriented
entry point to the lockfile API, and teach the lockfile code to handle
closing the FILE correctly when necessary.

But I didn't want to add even more changes to this patch series, so I am
working on this as a separate patch series. I hope to submit it soon.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu

  reply	other threads:[~2014-09-30 13:42 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-26 10:08 [PATCH v6 00/39] Lockfile correctness and refactoring Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 01/39] unable_to_lock_die(): rename function from unable_to_lock_index_die() Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 02/39] api-lockfile: revise and expand the documentation Michael Haggerty
2014-09-26 18:33   ` Junio C Hamano
2014-09-30 10:53     ` Michael Haggerty
2014-09-30 17:39       ` Junio C Hamano
2014-10-01  7:37         ` Michael Haggerty
2014-09-26 20:40   ` Junio C Hamano
2014-09-30 13:41     ` Michael Haggerty [this message]
2014-09-30 16:15       ` Jeff King
2014-09-30 17:47         ` Junio C Hamano
2014-10-01  8:11           ` Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 03/39] close_lock_file(): exit (successfully) if file is already closed Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 04/39] rollback_lock_file(): do not clear filename redundantly Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 05/39] rollback_lock_file(): exit early if lock is not active Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 06/39] rollback_lock_file(): set fd to -1 Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 07/39] lockfile: unlock file if lockfile permissions cannot be adjusted Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 08/39] hold_lock_file_for_append(): release lock on errors Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 09/39] lock_file(): always initialize and register lock_file object Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 10/39] lockfile.c: document the various states of lock_file objects Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 11/39] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 12/39] delete_ref_loose(): don't muck around in the lock_file's filename Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 13/39] prepare_index(): declare return value to be (const char *) Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 14/39] write_packed_entry_fn(): convert cb_data into a (const int *) Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 15/39] lock_file(): exit early if lockfile cannot be opened Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 16/39] remove_lock_file(): call rollback_lock_file() Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 17/39] commit_lock_file(): inline temporary variable Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 18/39] commit_lock_file(): die() if called for unlocked lockfile object Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 19/39] close_lock_file(): if close fails, roll back Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 20/39] commit_lock_file(): rollback lock file on failure to rename Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 21/39] api-lockfile: document edge cases Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 22/39] dump_marks(): remove a redundant call to rollback_lock_file() Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 23/39] git_config_set_multivar_in_file(): avoid " Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 24/39] lockfile: avoid transitory invalid states Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 25/39] struct lock_file: declare some fields volatile Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 26/39] try_merge_strategy(): remove redundant lock_file allocation Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 27/39] try_merge_strategy(): use a statically-allocated lock_file object Michael Haggerty
2014-09-26 19:00   ` Junio C Hamano
2014-09-30 14:04     ` Michael Haggerty
2014-09-30 18:08       ` Junio C Hamano
2014-09-26 10:08 ` [PATCH v6 28/39] commit_lock_file(): use a strbuf to manage temporary space Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 29/39] Change lock_file::filename into a strbuf Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 30/39] resolve_symlink(): use a strbuf for internal scratch space Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 31/39] resolve_symlink(): take a strbuf parameter Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 32/39] trim_last_path_component(): replace last_path_elm() Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 33/39] Extract a function commit_lock_file_to() Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 34/39] Rename LOCK_NODEREF to LOCK_NO_DEREF Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 35/39] lockfile.c: rename static functions Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 36/39] get_locked_file_path(): new function Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 37/39] hold_lock_file_for_append(): restore errno before returning Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 38/39] Move read_index() definition to read-cache.c Michael Haggerty
2014-09-26 10:08 ` [PATCH v6 39/39] lockfile.h: extract new header file for the functions in lockfile.c Michael Haggerty
2014-09-26 16:31 ` [PATCH v6 00/39] Lockfile correctness and refactoring Junio C Hamano
2014-09-30  9:55   ` Michael Haggerty
2014-09-26 19:10 ` Junio C Hamano

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=542AB323.4080402@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.