git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/8] midx: stop using linked list when closing MIDX

From
Patrick Steinhardt <ps@pks.im>
Date
Jul 15, 2025, 08:26 UTC
Message-ID
<aHYQyK0bu-RQcrHH@pks.im>
In-Reply-To
<aHBLUg7Y1cgf8i7k@nand.local>
On Thu, Jul 10, 2025 at 07:22:58PM -0400, Taylor Blau wrote:
Show 34 quoted lines
> On Wed, Jul 09, 2025 at 09:54:51AM +0200, Patrick Steinhardt wrote:
> > diff --git a/midx.c b/midx.c
> > index a91231bfcdf..416b3e8b54f 100644
> > --- a/midx.c
> > +++ b/midx.c
> > @@ -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;
> 
> ...and then this line would NULL the now-free()'d memory out.
> 
> But instead we are directly iterating through the sources and both
> closing and NULL-ing out their respective MIDXs (if any).
> 
> As an aside: I know we do the C99-style for loop with declarations in
> many places, but in this instance it seems to have produced an awfully
> long line. I wonder if in this instance it would be better to write:
> 
>     struct odb_source *source;
>     for (source = r->objects->sources; source; source = source->next) {
>         /* ... */
>     }
> 
> That's still a little lengthy, but it's fewer than 80 characters ;-).
Fair, can adapt.
Show 5 quoted lines
> > +		}
> > +		r->objects->multi_pack_index = NULL;
> 
> Presumably this pointer will go away at some point in the future as
> well?
Yup, exactly, it's removed at the end of this series.
Patrick
Previous: Taylor BlauNext: Patrick Steinhardt
Message 16 of 43 in “odb: track multi-pack-indices via their object sources”
  1. 0/8 odb: track multi-pack-indices via their object sourcesPatrick Steinhardt, Jul 9, 2025
  2. 1/8 midx: start tracking per object database sourcePatrick Steinhardt, Jul 9, 2025
  3. Justin ToblerJul 10, 2025
  4. Taylor BlauJul 10, 2025
  5. Taylor BlauJul 10, 2025
  6. Patrick SteinhardtJul 15, 2025
  7. Patrick SteinhardtJul 15, 2025
  8. 2/8 packfile: refactor `prepare_packed_git_one()` to work on sourcesPatrick Steinhardt, Jul 9, 2025
  9. Justin ToblerJul 10, 2025
  10. Taylor BlauJul 10, 2025
  11. Patrick SteinhardtJul 15, 2025
  12. 3/8 midx: stop using linked list when closing MIDXPatrick Steinhardt, Jul 9, 2025
  13. Justin ToblerJul 10, 2025
  14. Patrick SteinhardtJul 15, 2025
  15. Taylor BlauJul 10, 2025
  16. Patrick SteinhardtJul 15, 2025
  17. 4/8 midx: track whether we have loaded the MIDXPatrick Steinhardt, Jul 9, 2025
  18. Justin ToblerJul 10, 2025
  19. Taylor BlauJul 10, 2025
  20. Patrick SteinhardtJul 15, 2025
  21. 5/8 packfile: refactor `get_multi_pack_index()` to work on sourcesPatrick Steinhardt, Jul 9, 2025
  22. Justin ToblerJul 10, 2025
  23. Taylor BlauJul 10, 2025
  24. Patrick SteinhardtJul 15, 2025
  25. 6/8 packfile: stop using linked MIDX list in `find_pack_entry()`Patrick Steinhardt, Jul 9, 2025
  26. 7/8 packfile: stop using linked MIDX list in `get_all_packs()`Patrick Steinhardt, Jul 9, 2025
  27. 8/8 midx: remove now-unused linked list of multi-pack indicesPatrick Steinhardt, Jul 9, 2025
  28. Justin ToblerJul 10, 2025
  29. Junio C HamanoJul 9, 2025
  30. Taylor BlauJul 10, 2025
  31. Patrick SteinhardtJul 15, 2025
  32. 0/7 odb: track multi-pack-indices via their object sourcesPatrick Steinhardt, Jul 15, 2025
  33. 1/7 midx: start tracking per object database sourcePatrick Steinhardt, Jul 15, 2025
  34. 2/7 packfile: refactor `prepare_packed_git_one()` to work on sourcesPatrick Steinhardt, Jul 15, 2025
  35. 3/7 midx: stop using linked list when closing MIDXPatrick Steinhardt, Jul 15, 2025
  36. 4/7 packfile: refactor `get_multi_pack_index()` to work on sourcesPatrick Steinhardt, Jul 15, 2025
  37. 5/7 packfile: stop using linked MIDX list in `find_pack_entry()`Patrick Steinhardt, Jul 15, 2025
  38. 6/7 packfile: stop using linked MIDX list in `get_all_packs()`Patrick Steinhardt, Jul 15, 2025
  39. 7/7 midx: remove now-unused linked list of multi-pack indicesPatrick Steinhardt, Jul 15, 2025
  40. Justin ToblerJul 15, 2025
  41. Junio C HamanoJul 23, 2025
  42. Patrick SteinhardtJul 24, 2025
  43. Taylor BlauAug 12, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.