Re: [PATCH v3 15/24] read-cache: read index-v5

[Thomas Gummerer <t.gummerer@gmail.com>]

Duy Nguyen <pclouds@gmail.com> writes:

> On Mon, Aug 19, 2013 at 2:42 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
>> +static int read_entry(struct cache_entry **ce, char *pathname, size_t pathlen,
>> +                     void *mmap, unsigned long mmap_size,
>> +                     unsigned int first_entry_offset,
>> +                     unsigned int foffsetblock)
>> +{
>> +       int len, offset_to_offset;
>> +       char *name;
>> +       uint32_t foffsetblockcrc, *filecrc, *beginning, *end, entry_offset;
>> +       struct ondisk_cache_entry *disk_ce;
>> +
>> +       beginning = ptr_add(mmap, foffsetblock);
>> +       end = ptr_add(mmap, foffsetblock + 4);
>> +       len = ntoh_l(*end) - ntoh_l(*beginning) - sizeof(struct ondisk_cache_entry) - 5;
>> +       entry_offset = first_entry_offset + ntoh_l(*beginning);
>> +       name = ptr_add(mmap, entry_offset);
>> +       disk_ce = ptr_add(mmap, entry_offset + len + 1);
>> +       *ce = cache_entry_from_ondisk(disk_ce, pathname, name, len, pathlen);
>> +       filecrc = ptr_add(mmap, entry_offset + len + 1 + sizeof(*disk_ce));
>> +       offset_to_offset = htonl(foffsetblock);
>> +       foffsetblockcrc = crc32(0, (Bytef*)&offset_to_offset, 4);
>> +       if (!check_crc32(foffsetblockcrc,
>> +               ptr_add(mmap, entry_offset), len + 1 + sizeof(*disk_ce),
>> +               ntoh_l(*filecrc)))
>> +               return -1;
>> +
>> +       return 0;
>> +}
>
> Last thought before book+bed time. I wonder if moving the name part to
> the end of the entry (i.e. chaging on disk format) would simplify this
> code. The new ondisk_cache_entry would be something like this
>
> struct ondisk_cache_entry {
>    uint16_t flags;
>    uint16_t mode;
>    struct cache_time mtime;
>    uint32_t size;
>    int stat_crc;
>    unsigned char sha1[20];
>    char name[FLEX_ARRAY];
> };

I think it simplifies it a bit, but not too much, the only thing I see
avoiding the use of the name variable.  I think it will also simplify
the writing code a bit.  The only negative part would be for bisecting
the index, but that would still be possible, and only slightly more
complicated.  I'll give it a try tomorrow and check if it's worth it.