Jeff King <peff@xxxxxxxx> writes:
> 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@xxxxxxxx>
> ---
> 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 ...
> +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.
