@matheusbsbs / matheus-bagliano / commits / d3d9c51973

repack: convert "names" util bitfield to array

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 21, 2022 at 20:21 UTC d3d9c51973bf3187d8767969a0aa33abe79d6240

1 file changed +14 -8

builtin/repack.c

+14 -8

@@ -247,11 +247,15 @@ static struct {

247 {".idx"},

248 };

249

250 -static unsigned populate_pack_exts(char *name)

250 +struct generated_pack_data {

251 + char exts[ARRAY_SIZE(exts)];

252 +};

253 +

254 +static struct generated_pack_data *populate_pack_exts(const char *name)

255 {

256 struct stat statbuf;

257 struct strbuf path = STRBUF_INIT;

254 - unsigned ret = 0;

258 + struct generated_pack_data *data = xcalloc(1, sizeof(*data));

259 int i;

260

261 for (i = 0; i < ARRAY_SIZE(exts); i++) {

@@ -261,11 +265,11 @@ static unsigned populate_pack_exts(char *name)

265 if (stat(path.buf, &statbuf))

266 continue;

267

264 - ret |= (1 << i);

268 + data->exts[i] = 1;

269 }

270

271 strbuf_release(&path);

268 - return ret;

272 + return data;

273 }

274

275 static void repack_promisor_objects(const struct pack_objects_args *args,

@@ -320,7 +324,7 @@ static void repack_promisor_objects(const struct pack_objects_args *args,

324 line.buf);

325 write_promisor_file(promisor_name, NULL, 0);

326

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

327 + item->util = populate_pack_exts(item->string);

328

329 free(promisor_name);

330 }

@@ -994,7 +998,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)

998 string_list_sort(&names);

999

1000 for_each_string_list_item(item, &names) {

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

1001 + item->util = populate_pack_exts(item->string);

1002 }

1003

1004 close_object_store(the_repository->objects);

@@ -1003,6 +1007,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)

1007 * Ok we have prepared all new packfiles.

1008 */

1009 for_each_string_list_item(item, &names) {

1010 + struct generated_pack_data *data = item->util;

1011 +

1012 for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {

1013 char *fname, *fname_old;

1014

@@ -1011,7 +1017,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)

1017 fname_old = mkpathdup("%s-%s%s",

1018 packtmp, item->string, exts[ext].name);

1019

1014 - if (((uintptr_t)item->util) & ((uintptr_t)1 << ext)) {

1020 + if (data->exts[ext]) {

1021 struct stat statbuffer;

1022 if (!stat(fname_old, &statbuffer)) {

1023 statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);

@@ -1115,7 +1121,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)

1121 write_midx_file(get_object_directory(), NULL, NULL, flags);

1122 }

1123

1118 - string_list_clear(&names, 0);

1124 + string_list_clear(&names, 1);

1125 string_list_clear(&existing_nonkept_packs, 0);

1126 string_list_clear(&existing_kept_packs, 0);

1127 clear_pack_geometry(geometry);