On 25/07/09 09:54AM, Patrick Steinhardt wrote:
> When calling `close_midx()` we not only close the multi-pack index for
> one object source, but instead we iterate through the whole linked list
> of MIDXs to close all of them. This linked list is about to go away in
> favor of using the new per-source pointer to its respective MIDX.
>
> Refactor the function to iterate through sources instead.
The `close_midx()` function itself is not iterating though the sources.
Rather each of the callsites are now resposible to ensure `close_midx()`
is called separately for each source. It might be nice to clarify this a
bit in the message.
I also noticed that there are several other existing `close_midx()`
callsites that we leave as-is. Each of these sites though looks like
they don't care about globally closing all MIDXs so they should be fine.
This might also we worth mentioning.
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> midx.c | 11 ++++++-----
> packfile.c | 10 +++++-----
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/midx.c b/midx.c
> index a91231bfcdf..416b3e8b54f 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -401,7 +401,6 @@ void close_midx(struct multi_pack_index *m)
> if (!m)
> return;
>
> - close_midx(m->next);
By dropping this recursive call, now only the single MIDX chain is
closed. Other entries in the list are no longer recursively interated
through and will have to `close_midx()` called on them explicitly.
> close_midx(m->base_midx);
>
> munmap((unsigned char *)m->data, m->data_len);
> @@ -835,11 +834,13 @@ void clear_midx_file(struct repository *r)
>
> get_midx_filename(r->hash_algo, &midx, r->objects->sources->path);
>
> - if (r->objects && r->objects->multi_pack_index) {
> - close_midx(r->objects->multi_pack_index);
> - r->objects->multi_pack_index = NULL;
> - for (struct odb_source *source = r->objects->sources; source; source = source->next)
> + if (r->objects) {
> + for (struct odb_source *source = r->objects->sources; source; source = source->next) {
> + if (source->multi_pack_index)
> + close_midx(source->multi_pack_index);
> source->multi_pack_index = NULL;
> + }
> + r->objects->multi_pack_index = NULL;
At this callsite we want to close all of the loaded MIDX and do so my
iterating through each of the sources. Since we still have the global
MIDX for now, we explicitly unset it.
> }
>
> if (remove_path(midx.buf))
> diff --git a/packfile.c b/packfile.c
> index b43dd2fe6cb..546c161d0c1 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -369,12 +369,12 @@ void close_object_store(struct object_database *o)
> else
> close_pack(p);
>
> - if (o->multi_pack_index) {
> - close_midx(o->multi_pack_index);
> - o->multi_pack_index = NULL;
> - for (struct odb_source *source = o->sources; source; source = source->next)
> - source->multi_pack_index = NULL;
> + for (struct odb_source *source = o->sources; source; source = source->next) {
> + if (source->multi_pack_index)
> + close_midx(source->multi_pack_index);
> + source->multi_pack_index = NULL;
> }
> + o->multi_pack_index = NULL;
Same here. Looking good :)
-Justin
