From: Michael Haggerty <mhagger@alum.mit.edu>
To: Jeff King <peff@peff.net>, git@vger.kernel.org
Cc: Jim Hill <gjthill@gmail.com>
Subject: Re: [PATCH 04/17] add_to_alternates_file: don't add duplicate entries
Date: Tue, 11 Aug 2015 06:00:20 +0200 [thread overview]
Message-ID: <55C97354.7080008@alum.mit.edu> (raw)
In-Reply-To: <20150810093446.GD30981@sigill.intra.peff.net>
On 08/10/2015 11:34 AM, Jeff King wrote:
> The add_to_alternates_file function blindly uses
> hold_lock_file_for_append to copy the existing contents, and
> then adds the new line to it. This has two minor problems:
>
> 1. We might add duplicate entries, which are ugly and
> inefficient.
>
> 2. We do not check that the file ends with a newline, in
> which case we would bogusly append to the final line.
> This is quite unlikely in practice, though, as we call
> this function only from git-clone, so presumably we are
> the only writers of the file (and we always add a
> newline).
>
> Instead of using hold_lock_file_for_append, let's copy the
> file line by line, which ensures all records are properly
> terminated. If we see an extra line, we can simply abort the
> update (there is no point in even copying the rest, as we
> know that it would be identical to the original).
Do we have reason to expect that a lot of people have alternates files
that already contain duplicate lines? You say that this function is only
called from `git clone`, so I guess the answer is "no".
But if I'm wrong, it might be friendly to de-dup the existing lines
while copying them.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
next prev parent reply other threads:[~2015-08-11 4:01 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-10 9:27 [PATCH 0/17] removing questionable uses of git_path Jeff King
2015-08-10 9:32 ` [PATCH 01/17] cache.h: clarify documentation for git_path, et al Jeff King
2015-08-10 9:32 ` [PATCH 02/17] cache.h: complete set of git_path_submodule helpers Jeff King
2015-08-10 9:32 ` [PATCH 03/17] t5700: modernize style Jeff King
2015-08-10 9:34 ` [PATCH 04/17] add_to_alternates_file: don't add duplicate entries Jeff King
2015-08-11 4:00 ` Michael Haggerty [this message]
2015-08-11 9:54 ` Jeff King
2015-08-10 9:35 ` [PATCH 05/17] remove hold_lock_file_for_append Jeff King
2015-08-10 22:36 ` Junio C Hamano
2015-08-11 9:38 ` Jeff King
2015-08-10 9:35 ` [PATCH 06/17] prefer git_pathdup to git_path in some possibly-dangerous cases Jeff King
2015-08-10 9:35 ` [PATCH 07/17] prefer mkpathdup to mkpath in assignments Jeff King
2015-08-10 9:35 ` [PATCH 08/17] remote.c: drop extraneous local variable from migrate_file Jeff King
2015-08-10 9:36 ` [PATCH 09/17] refs.c: remove extra git_path calls from read_loose_refs Jeff King
2015-08-10 9:36 ` [PATCH 10/17] path.c: drop git_path_submodule Jeff King
2015-08-10 22:50 ` Junio C Hamano
2015-08-10 22:57 ` Junio C Hamano
2015-08-10 23:52 ` Junio C Hamano
2015-08-11 9:53 ` Jeff King
2015-08-10 9:36 ` [PATCH 11/17] refs.c: simplify strbufs in reflog setup and writing Jeff King
2015-08-10 10:34 ` Michael Haggerty
2015-08-10 12:26 ` Jeff King
2015-08-10 9:36 ` [PATCH 12/17] refs.c: avoid repeated git_path calls in rename_tmp_log Jeff King
2015-08-10 9:37 ` [PATCH 13/17] refs.c: avoid git_path assignment in lock_ref_sha1_basic Jeff King
2015-08-10 9:37 ` [PATCH 14/17] refs.c: remove_empty_directories can take a strbuf Jeff King
2015-08-10 9:37 ` [PATCH 15/17] find_hook: keep our own static buffer Jeff King
2015-08-10 9:37 ` [PATCH 16/17] get_repo_path: refactor path-allocation Jeff King
2015-08-10 9:38 ` [PATCH 17/17] memoize common git-path "constant" files Jeff King
2015-08-10 12:05 ` Michael Haggerty
2015-08-10 12:30 ` Jeff King
2015-08-10 12:06 ` [PATCH 0/17] removing questionable uses of git_path Michael Haggerty
2015-08-10 17:31 ` Junio C Hamano
2015-08-10 17:47 ` Jeff King
2015-08-15 9:05 ` Duy Nguyen
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=55C97354.7080008@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=git@vger.kernel.org \
--cc=gjthill@gmail.com \
--cc=peff@peff.net \
/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.