Re: [PATCH 09/44] object-store: free alt_odb_list
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 4, 2018, 02:37 UTC
- Message-ID
- <CAPig+cSkU8j-NiNeMP-F3VygF1sAoXP3qzn4fx7C1qxTZoO5+g@mail.gmail.com>
- In-Reply-To
- <20180303113637.26518-10-pclouds@gmail.com>
On Sat, Mar 3, 2018 at 6:36 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
Show 12 quoted lines
> 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?
Show 7 quoted lines
> +static void free_alt_odbs(struct raw_object_store *o)
> +{
> + while (o->alt_odb_list) {
> + free_alt_odb(o->alt_odb_list);
> + o->alt_odb_list = o->alt_odb_list->next;
> + }
> +}Accessing an entry's 'next' member after invoking free_alt_odb() works because the entry itself hasn't been freed (as noted above).
Is leaking the entries themselves intentional?
Show 8 quoted lines
> void raw_object_store_clear(struct raw_object_store *o)
> {
> FREE_AND_NULL(o->objectdir);
> FREE_AND_NULL(o->alternate_db);
> +
> + free_alt_odbs(o);
> + o->alt_odb_tail = NULL;
> }The commit message talks about freeing memory and resetting alt_odb_list and alt_odb_tail, but this code only resets alt_odb_tail.