On 03/03, Eric Sunshine wrote:
> On Sat, Mar 3, 2018 at 6:36 AM, Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> wrote:
> > Free the memory and reset alt_odb_{list, tail} to NULL.
> >
> > Signed-off-by: Stefan Beller <sbel...@google.com>
> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@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?
Yep it definitely should free the entry itself. Should be fixed easy
enough by freeing the list entry after grabbing the next entry
>
> > +static void free_alt_odbs(struct raw_object_store *o)
> > +{
> > + while (o->alt_odb_list) {
struct alternate_object_database old = o->alt_odb_list;
> > + free_alt_odb(o->alt_odb_list);
> > + o->alt_odb_list = o->alt_odb_list->next;
free(old);
> > + }
> > +}
>
> 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?
>
> > 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.
--
Brandon Williams
