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

>>> +static int read_entries(struct index_state *istate, struct directory_entry *de,
>>> +                       unsigned int first_entry_offset, void *mmap,
>>> +                       unsigned long mmap_size, unsigned int *nr,
>>> +                       unsigned int foffsetblock)
>>> +{
>>> +       struct cache_entry *ce;
>>> +       int i, subdir = 0;
>>> +
>>> +       for (i = 0; i < de->de_nfiles; i++) {
>>> +               unsigned int subdir_foffsetblock = de->de_foffset + foffsetblock + (i * 4);
>>> +               if (read_entry(&ce, de->pathname, de->de_pathlen, mmap, mmap_size,
>>> +                              first_entry_offset, subdir_foffsetblock) < 0)
>>> +                       return -1;
>>
>> You read one file entry, say abc/def...
>
> You're not quite right here.  I'm reading def here, de is the root
> directory and de->sub[subdir] is the first sub directory, named abc/
>
>>> +               while (subdir < de->de_nsubtrees &&
>>> +                      cache_name_compare(ce->name + de->de_pathlen,
>>> +                                         ce_namelen(ce) - de->de_pathlen,
>>> +                                         de->sub[subdir]->pathname + de->de_pathlen,
>>> +                                         de->sub[subdir]->de_pathlen - de->de_pathlen) > 0) {
>>
>> Oh right the entry belongs the the substree "abc" so..
>
> abc/ comes before def, so lets read everything in that directory first.
>
>>> +                       read_entries(istate, de->sub[subdir], first_entry_offset, mmap,
>>> +                                    mmap_size, nr, foffsetblock);
>>
>> you recurse in, which will add following entries like abc/def and abc/xyz...
>
> Recurse in, add abc/def and abc/xyz, and increase nr in the recursion,
> so the new entry gets added at the right place.
>
>>> +                       subdir++;
>>> +               }
>>> +               if (!ce)
>>> +                       continue;
>>> +               set_index_entry(istate, (*nr)++, ce);
>>
>> then back here after recusion and add abc/def, again, after abc/xyz.
>> Did I read this code correctly?
>
> After the recursion add def to at the 3rd position in the index.  After
> that it looks like:
> abc/def
> abc/xyz
> def
>
> I hope that makes it a little clearer.

>>> +       de = root_directory;
>>> +       last_de = de;
>>> +       while (de) {
>>> +               if (need_root ||
>>> +                   match_pathspec_depth(&adjusted_pathspec, de->pathname, de->de_pathlen, 0, NULL)) {
>>> +                       if (read_entries(istate, de, entry_offset,
>>> +                                        mmap, mmap_size, &nr,
>>> +                                        foffsetblock) < 0)
>>> +                               return -1;
>>> +               } else {
>>> +                       for (i = 0; i < de->de_nsubtrees; i++) {
>>> +                               last_de->next = de->sub[i];
>>> +                               last_de = last_de->next;
>>> +                       }
>>> +               }
>>> +               de = de->next;
>>
>> I'm missing something here. read_entries is a function that reads all
>> entries inside "de" including subdirectories and the first "de" is
>> root_directory, which makes it read the whole index in.
>
> It does, except when the adjusted_pathspec doesn't match the
> root_directory.  In that case all the subdirectories of the
> root_directory are added to a queue, which will then be iterated over
> and tried to match with the adjusted_pathspec.