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

Re: [PATCH 1/4] repack: convert "names" util bitfield to array

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 21, 2022, 22:19 UTC
Message-ID
<xmqqtu3wizuc.fsf@gitster.g>
In-Reply-To
<Y1MSWAx+baTklfLL@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 36 quoted lines
> We keep a string_list "names" containing the hashes of packs generated
> on our behalf by pack-objects. The util field of each item is treated as
> a bitfield that tells us which extensions (.pack, .idx, .rev, etc) are
> present for each name.
>
> Let's switch this to allocating a real array. That will give us room in
> a future patch to store more data than just a single bit per extension.
> And it makes the code a little easier to read, as we avoid casting back
> and forth between uintptr_t and a void pointer.
>
> Since the only thing we're storing is an array, we could just allocate
> it directly. But instead I've put it into a named struct here. That
> further increases readability around the casts, and in particular helps
> differentiate us from other string_lists in the same file which use
> their util field differently. E.g., the existing_*_packs lists still do
> bit-twiddling, but their bits have different meaning than the ones in
> "names". This makes it hard to grep around the code to see how the util
> fields are used; now you can look for "generated_pack_data".
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/repack.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/repack.c b/builtin/repack.c
> index a5bacc7797..8e71230bf7 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -247,11 +247,15 @@ static struct {
>  	{".idx"},
>  };
>  
> -static unsigned populate_pack_exts(char *name)
> +struct generated_pack_data {
> +	char exts[ARRAY_SIZE(exts)];
> +};

OK, so instead of a single "unsigned" word holding six bits (the number of elements in the exts[] array), we have one byte per the file extension. Since ...

Show 6 quoted lines
> +static struct generated_pack_data *populate_pack_exts(const char *name)
>  {
>  	struct stat statbuf;
>  	struct strbuf path = STRBUF_INIT;
> -	unsigned ret = 0;
> +	struct generated_pack_data *data = xcalloc(1, sizeof(*data));

... this is allocated a real piece of storage and the function returns a pointer to it, ...

> -		item->util = (void *)(uintptr_t)populate_pack_exts(item->string);
> +		item->util = populate_pack_exts(item->string);

... we no longer need to cast but the value can go straight to the .util member, and ...

> -	string_list_clear(&names, 0);
> +	string_list_clear(&names, 1);

... we now need to free these structs pointed at by the .util member.

All make sense.
Previous: Jeff KingNext: Taylor Blau
Message 3 of 34 in “repack tempfile-cleanup signal deadlock”
  1. 0/4 repack tempfile-cleanup signal deadlockJeff King, Oct 21, 2022
  2. 1/4 repack: convert "names" util bitfield to arrayJeff King, Oct 21, 2022
  3. Junio C HamanoOct 21, 2022
  4. Taylor BlauOct 21, 2022
  5. Jeff KingOct 21, 2022
  6. Junio C HamanoOct 21, 2022
  7. Jeff KingOct 21, 2022
  8. Junio C HamanoOct 21, 2022
  9. Jeff KingOct 22, 2022
  10. 2/4 repack: populate extension bits incrementallyJeff King, Oct 21, 2022
  11. Taylor BlauOct 21, 2022
  12. Jeff KingOct 21, 2022
  13. Taylor BlauOct 21, 2022
  14. Jeff KingOct 21, 2022
  15. 3/4 repack: use tempfiles for signal cleanupJeff King, Oct 21, 2022
  16. Junio C HamanoOct 21, 2022
  17. Jeff KingOct 21, 2022
  18. Taylor BlauOct 21, 2022
  19. Jeff KingOct 22, 2022
  20. Jeff KingOct 22, 2022
  21. 4/4 repack: drop remove_temporary_files()Jeff King, Oct 21, 2022
  22. Taylor BlauOct 21, 2022
  23. 0/5 repack tempfile-cleanup signal deadlockJeff King, Oct 22, 2022
  24. 1/5 repack: convert "names" util bitfield to arrayJeff King, Oct 22, 2022
  25. 2/5 repack: populate extension bits incrementallyJeff King, Oct 22, 2022
  26. 3/5 repack: expand error message for missing pack filesJeff King, Oct 22, 2022
  27. 4/5 repack: use tempfiles for signal cleanupJeff King, Oct 22, 2022
  28. Jeff KingOct 22, 2022
  29. Junio C HamanoOct 23, 2022
  30. Jeff KingOct 23, 2022
  31. Junio C HamanoOct 23, 2022
  32. Jeff KingOct 23, 2022
  33. Junio C HamanoOct 23, 2022
  34. 5/5 repack: drop remove_temporary_files()Jeff King, Oct 22, 2022

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.