Re: [PATCH 03/16] refs: implement releasing ref storages

From: Patrick Steinhardt <ps@pks.im>
Date: May 17, 2024, 07:08 UTC
Message-ID: <ZkcCh3OhbPOn0HOF@tanuki>
In-Reply-To: <CAOLa=ZT3VqhrHXt9x-4DN7norTb3RuscBx+VHipn=QxJvwwThQ@mail.gmail.com>

On Thu, May 16, 2024 at 11:39:36AM -0500, Karthik Nayak wrote:

Show 17 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> [snip]
> 
> > diff --git a/refs/debug.c b/refs/debug.c
> > index 58fb4557ed..27aae42134 100644
> > --- a/refs/debug.c
> > +++ b/refs/debug.c
> > @@ -33,6 +33,12 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor
> >  	return (struct ref_store *)res;
> >  }
> >
> > +static void debug_release(struct ref_store *refs)
> > +{
> > +	struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
> 
> We should probably add a trace here, using `trace_printf_key()`

I didn't because we don't have `debug_init()` with a trace, either, and because there is no error code that we could report. But on the other hand it does not hurt to have it here, so let's just add it.

[snip]

Show 24 quoted lines

> > diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> > index a4bb71cd76..6c262c2193 100644
> > --- a/refs/reftable-backend.c
> > +++ b/refs/reftable-backend.c
> > @@ -293,6 +293,27 @@ static struct ref_store *reftable_be_init(struct repository *repo,
> >  	return &refs->base;
> >  }
> >
> > +static void reftable_be_release(struct ref_store *ref_store)
> > +{
> > +	struct reftable_ref_store *refs = reftable_be_downcast(ref_store, 0, "release");
> > +	struct strmap_entry *entry;
> > +	struct hashmap_iter iter;
> > +
> > +	if (refs->main_stack) {
> > +		reftable_stack_destroy(refs->main_stack);
> > +		refs->main_stack = NULL;
> > +	}
> > +
> > +	if (refs->worktree_stack) {
> > +		reftable_stack_destroy(refs->worktree_stack);
> > +		refs->main_stack = NULL;
> 
> This should be `refs->worktree_stack`, right?

Good catch, yes.

Patrick