Re: [PATCH 09/44] object-store: free alt_odb_list

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 5, 2018, 09:54 UTC
Message-ID: <CACsJy8AH08k-=DATSn63T_4uVmjNoGn8m6k4=ZTq5RbO386k7A@mail.gmail.com>
In-Reply-To: <CAPig+cSkU8j-NiNeMP-F3VygF1sAoXP3qzn4fx7C1qxTZoO5+g@mail.gmail.com>

On Sun, Mar 4, 2018 at 9:37 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 17 quoted lines

> On Sat, Mar 3, 2018 at 6:36 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> Free the memory and reset alt_odb_{list, tail} to NULL.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> diff --git a/object.c b/object.c
>> @@ -450,8 +450,26 @@ void raw_object_store_init(struct raw_object_store *o)
>> +static void free_alt_odb(struct alternate_object_database *alt)
>> +{
>> +       strbuf_release(&alt->scratch);
>> +       oid_array_clear(&alt->loose_objects_cache);
>> +}
>
> This doesn't free the 'struct alternate_object_database' entry itself, right?
>
> Is that intentional? Isn't the idea that this should free the entries too?

I think "alt" should be freed too. But since this series is more about refactoring, and we're still in the middle of converting to struct repository/raw_object_store, I don't mind "leaking a bit" (the current state is leaking freely when most things are global). Once the conversion is done, I think we could have some nice test that creates and destroys the object store gracefully, then tools like valgrind. asan... can help clean up these leaks.

-- 
Duy