Re: [PATCH 31/35] pathspec: allow querying for attributes

[Stefan Beller <sbeller@google.com>]

On Mon, Nov 28, 2016 at 2:11 PM, Brandon Williams <bmwill@google.com> wrote:
> On 11/10, Stefan Beller wrote:
>> @@ -500,6 +586,18 @@ void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
>>
>>  void clear_pathspec(struct pathspec *pathspec)
>>  {
>> +     int i, j;
>> +     for (i = 0; i < pathspec->nr; i++) {
>> +             if (!pathspec->items[i].attr_match_nr)
>> +                     continue;
>> +             for (j = 0; j < pathspec->items[j].attr_match_nr; j++)
>> +                     free(pathspec->items[i].attr_match[j].value);
>> +             free(pathspec->items[i].attr_match);
>> +             if (pathspec->items[i].attr_check)
>> +                     git_attr_check_clear(pathspec->items[i].attr_check);
>> +             free(pathspec->items[i].attr_check);
>> +     }
>> +
>>       free(pathspec->items);
>>       pathspec->items = NULL;
>
> You may also want to add logic like this to the 'copy_pathspec' function
> so that when a pathspec struct is copied, the destination also has
> ownership of its own attribute items.
>

Thanks for the review comments, I'll plan on resending this series after the
submodule checkout series (which is nowhere near done, but the foundation
is set with sb/submodule-intern-gitdir as a preparation.)

After discussion with Jonathan Nieder, I think it may be possible to make
this new pathspec magic work without the need for thread safety. This is
archived by constructing all relevant attr_check stacks before the preload_index
functionality (which is threaded) and then use the preconstructed attr_checks
to get attr_results just like in this series.

That approach would come with the benefit of not needing mutexes
at all inside the attr code.