@matheusbsbs / matheus-bagliano / commits / fba68184b8

midx: factor out `fill_pack_info()`

When selecting which packfiles will be written while generating a MIDX, the MIDX internals fill out a 'struct pack_info' with various pieces of book-keeping. Instead of filling out each field of the `pack_info` structure individually in each of the two spots that modify the array of such structures (`ctx->info`), extract a common routine that does this for us. This reduces the code duplication by a modest amount. But more importantly, it zero-initializes the structure before assigning values into it. This hardens us for a future change which will add additional fields to this structure which (until this patch) was not zero-initialized. As a result, any new fields added to the `pack_info` structure need only be updated in a single location, instead of at each spot within midx.c. There are no functional changes in this patch. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Dec 14, 2023 at 17:23 UTC fba68184b8de477934d1642ac1c311cfcf0f6041
1 file changed +20 -18
midx.c
+20 -18
@@ -475,6 +475,17 @@ struct pack_info {
475 unsigned expired : 1;
476 };
477
478 +static void fill_pack_info(struct pack_info *info,
479 + struct packed_git *p, const char *pack_name,
480 + uint32_t orig_pack_int_id)
481 +{
482 + memset(info, 0, sizeof(struct pack_info));
483 +
484 + info->orig_pack_int_id = orig_pack_int_id;
485 + info->pack_name = xstrdup(pack_name);
486 + info->p = p;
487 +}
488 +
489 static int pack_info_compare(const void *_a, const void *_b)
490 {
491 struct pack_info *a = (struct pack_info *)_a;
@@ -515,6 +526,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
526 const char *file_name, void *data)
527 {
528 struct write_midx_context *ctx = data;
529 + struct packed_git *p;
530
531 if (ends_with(file_name, ".idx")) {
532 display_progress(ctx->progress, ++ctx->pack_paths_checked);
@@ -541,27 +553,22 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
553
554 ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc);
555
544 - ctx->info[ctx->nr].p = add_packed_git(full_path,
545 - full_path_len,
546 - 0);
547 -
548 - if (!ctx->info[ctx->nr].p) {
556 + p = add_packed_git(full_path, full_path_len, 0);
557 + if (!p) {
558 warning(_("failed to add packfile '%s'"),
559 full_path);
560 return;
561 }
562
554 - if (open_pack_index(ctx->info[ctx->nr].p)) {
563 + if (open_pack_index(p)) {
564 warning(_("failed to open pack-index '%s'"),
565 full_path);
557 - close_pack(ctx->info[ctx->nr].p);
558 - FREE_AND_NULL(ctx->info[ctx->nr].p);
566 + close_pack(p);
567 + free(p);
568 return;
569 }
570
562 - ctx->info[ctx->nr].pack_name = xstrdup(file_name);
563 - ctx->info[ctx->nr].orig_pack_int_id = ctx->nr;
564 - ctx->info[ctx->nr].expired = 0;
571 + fill_pack_info(&ctx->info[ctx->nr], p, file_name, ctx->nr);
572 ctx->nr++;
573 }
574 }
@@ -1321,11 +1328,6 @@ static int write_midx_internal(const char *object_dir,
1328 for (i = 0; i < ctx.m->num_packs; i++) {
1329 ALLOC_GROW(ctx.info, ctx.nr + 1, ctx.alloc);
1330
1324 - ctx.info[ctx.nr].orig_pack_int_id = i;
1325 - ctx.info[ctx.nr].pack_name = xstrdup(ctx.m->pack_names[i]);
1326 - ctx.info[ctx.nr].p = ctx.m->packs[i];
1327 - ctx.info[ctx.nr].expired = 0;
1328 -
1331 if (flags & MIDX_WRITE_REV_INDEX) {
1332 /*
1333 * If generating a reverse index, need to have
@@ -1341,10 +1343,10 @@ static int write_midx_internal(const char *object_dir,
1343 if (open_pack_index(ctx.m->packs[i]))
1344 die(_("could not open index for %s"),
1345 ctx.m->packs[i]->pack_name);
1344 - ctx.info[ctx.nr].p = ctx.m->packs[i];
1346 }
1347
1347 - ctx.nr++;
1348 + fill_pack_info(&ctx.info[ctx.nr++], ctx.m->packs[i],
1349 + ctx.m->pack_names[i], i);
1350 }
1351 }
1352