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

2 messages, 2 authors, 2018-05-10 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2018-05-10 09:27:57

Stefan Beller [off-list ref] writes:
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@google.com/

We could just deal with all the exceptions, but that makes repo_clear
ugly IMHO.
So perhaps

         void repo_clear(...)
         {
        +       if (repo == the_repository)
        +               BUG("repo_clear() called on the repository");
                ...

or something?

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

From: Stefan Beller <hidden>
Date: 2018-05-10 16:34:18

Hi Junio,

On Thu, May 10, 2018 at 2:27 AM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
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@google.com/

We could just deal with all the exceptions, but that makes repo_clear
ugly IMHO.
So perhaps

         void repo_clear(...)
         {
        +       if (repo == the_repository)
        +               BUG("repo_clear() called on the repository");
                ...

or something?
This would work, but Duy convinced me to have repo_clear working
on the_repository is a good idea by giving a minimal test helper[1],
which helped me to find leaks[2][3], so I'd rather go with the solution
by Duy in [4] from a developers perspective.

Stefan

[1] https://public-inbox.org/git/CACsJy8C7N2W821H8YR8VaKdCSOSCDtQi_YT7z8hHNDO-VxJmEA@mail.gmail.com/
    https://gist.github.com/pclouds/86a2df6c28043f1b6fa3d4e72e7a1276
[2] https://public-inbox.org/git/20180510001211.163692-1-sbeller@google.com/
[3] https://public-inbox.org/git/20180509234059.52156-1-sbeller@google.com/
[4] https://public-inbox.org/git/CACsJy8AdJcQpiGrR3p6xfuqU0=qoP3=StgbWRNCkdfka6di+5w@mail.gmail.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help