Re: [PATCH] repository: fix free problem with repo_clear(the_repository)

[Stefan Beller <sbeller@google.com>]

On Wed, May 9, 2018 at 11:00 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, May 9, 2018 at 7:50 PM, Stefan Beller <sbeller@google.com> wrote:
>>>  I was trying to test the new parsed_object_pool_clear() and found this.
>>
>> So this would go with the latest sb/object-store-alloc ?
>
> No this should be separate because sb/object-store-alloc did not even
> touch this code. I mistakenly thought you wrote this and sent to you.
> I should have checked and sent to Brandon instead.

Yes, they do not produce merge conflicts and do not semantically
rely on each other. Except as noted below
the object store series complicated things in a non-latest version
of it, such that we'd have to add more special casing. Now everything
is fine.


>> I would rather special case the_repository even more instead
>> of having it allocate all its things on the heap. (However we rather
>> want to profile it and argue with data....)
>
> I'm actually going the opposite direction and trying hard to make
> the_repository normal like everybody else :)

ok, both Brandon and you want to do this, so I guess we'll just
go this route for now.

>
> discard_index(repo->index);
> if (repo->index != &the_index)
>         FREE_AND_NULL(repo->index);
>
>> What is your use case of repo_clear(the_repository)?
>
> No actual use case right now. See [1] for the code that triggered
> this. I do want to free some memory in pack-objects and repo_clear()
> _might_ be the one. I'm not sure yet.

This diff seems good to me. as any repo is cleared and wil have the minimal
amount of memory. the_repository clears its the_index which is also fine
as it has the minimal amount of memory then, too

Thanks,
Stefan