On Wed, May 9, 2018 at 10:04 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote: > the_repository is special. One of the special things about it is that > it does not allocate a new index_state object like submodules but > points to the global the_index variable instead. As a global variable, > the_index cannot be free()'d. ok. That is the situation we're in. > > Add an exception for this in repo_clear(). In the future perhaps we > would be able to allocate the_repository's index on heap too. Then we > can remove revert this. > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> > --- > 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 ? My impression was that we never call repo_clear() on the_repository, which would allow us to special case the_repository further just as I did in v2 of that series[1] by having static allocations for certain objects in case of \ the_repository. [1] https://public-inbox.org/git/20180501213403.14643-14-sbeller@xxxxxxxxxx/ We could just deal with all the exceptions, but that makes repo_clear ugly IMHO. 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....) > repository.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/repository.c b/repository.c > index a4848c1bd0..f44733524a 100644 > --- a/repository.c > +++ b/repository.c > @@ -238,7 +238,9 @@ void repo_clear(struct repository *repo) > > if (repo->index) { > discard_index(repo->index); > - FREE_AND_NULL(repo->index); > + if (repo->index != &the_index) > + free(repo->index); > + repo->index = NULL; So after this we have a "dangling" the_index. It is not really dangling, but it is not part of the_repository any more and many places still use the_index, it might make up for interesting bugs? What is your use case of repo_clear(the_repository)? Thanks, Stefan
