Re: [PATCH v7 2/2] Verify index file before we opportunistically update it

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 11, 2014, 02:57 UTC
Message-ID: <CACsJy8A5VJM4gnJGazkzniv5YxRx0xR7YEFpQbU+kVGofQ1b_g@mail.gmail.com>
In-Reply-To: <xmqqppkpvv9m.fsf@gitster.dls.corp.google.com>

On Fri, Apr 11, 2014 at 2:28 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

> Yiannis Marangos <yiannis.marangos@gmail.com> writes:
>
>> +     n = xpread(fd, sha1, 20, st.st_size - 20);
>> +     if (n != 20)
>> +             goto out;
>
> I think it is possible for pread(2) to give you a short-read.
>
> The existing callers of emulated mmap and index-pack are prepared to
> handle a short-read correctly, but I do not think this code does.
>
> I'll queue this instead in the meantime.

There are two things to sort out (sorry I can't spend much time on it right now): should the same sha1 test be done in write_index(), in addition to update_index_if_able(). And what do we do about istate->sha1[] in discard_index(), keep it or clear it.

-- 
Duy