Re: [PATCH v2 0/4] Fix mem leaks of recent object store conversions.

2018-05-11 Thread Stefan Beller
On Fri, May 11, 2018 at 1:37 AM, Jeff King  wrote:
> On Thu, May 10, 2018 at 12:58:45PM -0700, Stefan Beller wrote:
>
>> This series replaces the two commits that were queued on 
>> sb/object-store-replace,
>> fixing memory leaks that were recently introduced.
>>
>> Compared to v1, I merged the two independent series from yesterday,
>> rewrote the commit message to clear up Junios confusion and addresses Peffs
>> comments for the packfiles as well.
>
> Mostly. :)
>
> My one remaining complaint is that the bitmap code may hold on to a
> dangling pointer to a packed_git after this series.

Ok, I'll look into that.

>
> I think that is part of a larger problem, though, which is that the
> bitmap code's globals need to be part of the struct raw_object_store.
> I think this can already cause problems before your series if we were to
> try to use bitmaps in both a superproject and a submodule in the same
> process, though I think we'd at least hit the "ignoring extra bitmap
> file" code path in open_pack_bitmap_1(). So right now it's an annoyance,
> but after your series it becomes a potential segfault.

Ok, maybe we'll need to convert bitmaps into the object store for that.

Thanks for the pointer,
Stefan


Re: [PATCH v2 0/4] Fix mem leaks of recent object store conversions.

2018-05-11 Thread Jeff King
On Thu, May 10, 2018 at 12:58:45PM -0700, Stefan Beller wrote:

> This series replaces the two commits that were queued on 
> sb/object-store-replace,
> fixing memory leaks that were recently introduced.
> 
> Compared to v1, I merged the two independent series from yesterday,
> rewrote the commit message to clear up Junios confusion and addresses Peffs
> comments for the packfiles as well.

Mostly. :)

My one remaining complaint is that the bitmap code may hold on to a
dangling pointer to a packed_git after this series.

I think that is part of a larger problem, though, which is that the
bitmap code's globals need to be part of the struct raw_object_store.
I think this can already cause problems before your series if we were to
try to use bitmaps in both a superproject and a submodule in the same
process, though I think we'd at least hit the "ignoring extra bitmap
file" code path in open_pack_bitmap_1(). So right now it's an annoyance,
but after your series it becomes a potential segfault.

-Peff