Re: [PATCH 04/17] add_to_alternates_file: don't add duplicate entries
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Aug 11, 2015, 04:00 UTC
- Message-ID
- <55C97354.7080008@alum.mit.edu>
- In-Reply-To
- <20150810093446.GD30981@sigill.intra.peff.net>
On 08/10/2015 11:34 AM, Jeff King wrote:
Show 19 quoted lines
> 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