Re: [RFC PATCH v10 32.5/36] unpack_trees: fix memory corruption with split_index when src != dst

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sun, Apr 22, 2018 at 5:38 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
>>   - there's a better, more performant fix or there is some way to actually
>>     share a split_index between two independent index_state objects.
>
> A cleaner way of doing this would be something to the line [1]
>
>     move_index_extensions(&o->result, o->dst_index);
>
> near the end of this function. This could be where we compare the
> result index with the source index's shared file and see if it's worth
> keeping the shared index or not. Shared index is designed to work with
> huge index files though, any operations that go through all index
> entries will usually not be cheap. But at least it's safer.

Yeah, it looks like move_index_extensions() currently has no logic for
the split_index.  Adding it sounds to me like a patch series of its
own, and I'm keen to limit additional changes since my patch series
already broke things pretty badly once already.

>> However, with this fix, all the tests pass both normally and under
>> GIT_TEST_SPLIT_INDEX=DareISayYes.  Without this patch, when
>> GIT_TEST_SPLIT_INDEX is set, my directory rename detection series will fail
>> several tests, as reported by SZEDER.
>
> Yes, the change looks good.

Great, thanks for looking over it.

> [1] To me the second parameter should be src_index, not dst_index.
> We're copying entries from _source_ index to "result" and we should
> also copy extensions from the source index. That line happens to work
> only when dst_index is the same as src_index, which is the common use
> case so far.

That makes sense; this sounds like another fix that should be
submitted.  Did you want to submit a patch making that change?  Do you
want me to?

Elijah



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux