Re: [PATCH v7 3/7] eoie: add End of Index Entry (EOIE) extension
- From
Ben Peart <peartben@gmail.com>
- Date
- Oct 2, 2018, 15:13 UTC
- Message-ID
- <424326cc-0cfe-3af8-d2f7-6c124cb077cd@gmail.com>
- In-Reply-To
- <CACsJy8CX0TwVydzmqsjHK+W7tcaDgRCgeU3Gmc-bA1Ecf=Yz6A@mail.gmail.com>
On 10/1/2018 11:30 AM, Duy Nguyen wrote:
Show 10 quoted lines
> On Mon, Oct 1, 2018 at 3:46 PM Ben Peart <peartben@gmail.com> wrote: >> @@ -2479,6 +2491,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, >> if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0) >> return -1; >> >> + offset = lseek(newfd, 0, SEEK_CUR) + write_buffer_len; > > Note, lseek() could in theory return -1 on error. Looking at the error > code list in the man page it's pretty unlikely though, unless >
Good catch. I'll add the logic to check for an error.
Show 27 quoted lines
>> +static size_t read_eoie_extension(const char *mmap, size_t mmap_size)
>> +{
>> + /*
>> + * The end of index entries (EOIE) extension is guaranteed to be last
>> + * so that it can be found by scanning backwards from the EOF.
>> + *
>> + * "EOIE"
>> + * <4-byte length>
>> + * <4-byte offset>
>> + * <20-byte hash>
>> + */
>> + const char *index, *eoie;
>> + uint32_t extsize;
>> + size_t offset, src_offset;
>> + unsigned char hash[GIT_MAX_RAWSZ];
>> + git_hash_ctx c;
>> +
>> + /* ensure we have an index big enough to contain an EOIE extension */
>> + if (mmap_size < sizeof(struct cache_header) + EOIE_SIZE_WITH_HEADER + the_hash_algo->rawsz)
>
> Using sizeof() for on-disk structures could be dangerous because you
> don't know how much padding there could be (I'm not sure if it's
> actually specified in the C language spec). I've checked, on at least
> x86 and amd64, sizeof(struct cache_header) is 12 bytes, but I don't
> know if there are any crazy architectures out there that set higher
> padding.
> This must be safe as the same code has been in do_read_index() and verify_index_from() for a long time.