Re: [PATCH 6/7] repository: adapt `repo_clear()` to fully reset the repository

From: Patrick Steinhardt <ps@pks.im>
Date: Sep 28, 2026, 09:52 UTC
Message-ID: <aro4xkSiJDWknI1W@pks.im>
In-Reply-To: <CAOLa=ZQ_+Ofya1q01fpZjd_wDn=tk8YxbQWNxhFHya47hFRp-Q@mail.gmail.com>

On Mon, Sep 28, 2026 at 09:18:52AM +0000, Karthik Nayak wrote:

Show 17 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/repository.c b/repository.c
> > index b857e1c580..e67ff00550 100644
> > --- a/repository.c
> > +++ b/repository.c
> > @@ -439,6 +436,8 @@ void repo_clear(struct repository *repo)
> >  	strmap_clear(&repo->worktree_ref_stores, 1);
> >
> >  	repo_clear_path_cache(&repo->cached_paths);
> > +
> > +	memset(repo, 0, sizeof(*repo));
> 
> The reason we swap `FREE_AND_NULL()` with `free()` is because we anyways
> set everything to 0. Okay.
> 
> Or was this referring to the 'already blank' repository? Since
> FREE_AND_NULL() can already handle NULL values.

Yeah, the only reason I swap to plain free(3p) calls is because it's redundant now with the final call to memset(3p). I think the part about already-blank repositories is not accurate anymore, but it used to be at one point. Let me reword it.

Show 21 quoted lines

> > diff --git a/repository.h b/repository.h
> > index 11f5c2ed10..2a348012e8 100644
> > --- a/repository.h
> > +++ b/repository.h
> > @@ -258,6 +258,7 @@ void repo_set_ref_storage_format(struct repository *repo,
> >  void initialize_repository(struct repository *repo);
> >  RESULT_MUST_BE_USED
> >  int repo_init(struct repository *r, const char *gitdir, const char *worktree);
> > +void repo_clear(struct repository *repo);
> >
> >  /*
> >   * Initialize the repository 'subrepo' as the submodule at the given path. If
> > @@ -273,7 +274,6 @@ int repo_submodule_init(struct repository *subrepo,
> >  			struct repository *superproject,
> >  			const char *path,
> >  			const struct object_id *treeish_name);
> > -void repo_clear(struct repository *repo);
> >
> 
> This is a purely cosmetic move to bring it closer to `repo_init()`,
> right? I think it makes sense.

Yes, it is.

Patrick