@matheusbsbs / matheus-bagliano / commits / 66833f0e70

pack-write: refactor renaming in finish_tmp_packfile()

Refactor the renaming in finish_tmp_packfile() into a helper function. The callers are now expected to pass a "name_buffer" ending in "pack-OID." instead of the previous "pack-", we then append "pack", "idx" or "rev" to it. By doing the strbuf_setlen() in rename_tmp_packfile() we reuse the buffer and avoid the repeated allocations we'd get if that function had its own temporary "struct strbuf". This approach of reusing the buffer does make the last user in pack-object.c's write_pack_file() slightly awkward, since we needlessly do a strbuf_setlen() before calling strbuf_release() for consistency. In subsequent changes we'll move that bitmap writing code around, so let's not skip the strbuf_setlen() now. The previous strbuf_reset() idiom originated with 5889271114a (finish_tmp_packfile():use strbuf for pathname construction, 2014-03-03), which in turn was a minimal adjustment of pre-strbuf code added in 0e990530ae (finish_tmp_packfile(): a helper function, 2011-10-28). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Sep 9, 2021 at 19:24 UTC 66833f0e70c473ca6c4e6a79d34e879d8b40ba9d

3 files changed +23 -24

builtin/pack-objects.c

+5 -2

@@ -1237,7 +1237,8 @@ static void write_pack_file(void)

1237 warning_errno(_("failed utime() on %s"), pack_tmp_name);

1238 }

1239

1240 - strbuf_addf(&tmpname, "%s-", base_name);

1240 + strbuf_addf(&tmpname, "%s-%s.", base_name,

1241 + hash_to_hex(hash));

1242

1243 if (write_bitmap_index) {

1244 bitmap_writer_set_checksum(hash);

@@ -1250,8 +1251,9 @@ static void write_pack_file(void)

1251 &pack_idx_opts, hash);

1252

1253 if (write_bitmap_index) {

1253 - strbuf_addf(&tmpname, "%s.bitmap", hash_to_hex(hash));

1254 + size_t tmpname_len = tmpname.len;

1255

1256 + strbuf_addstr(&tmpname, "bitmap");

1257 stop_progress(&progress_state);

1258

1259 bitmap_writer_show_progress(progress);

@@ -1260,6 +1262,7 @@ static void write_pack_file(void)

1262 bitmap_writer_finish(written_list, nr_written,

1263 tmpname.buf, write_bitmap_options);

1264 write_bitmap_index = 0;

1265 + strbuf_setlen(&tmpname, tmpname_len);

1266 }

1267

1268 strbuf_release(&tmpname);

bulk-checkin.c

+2 -1

@@ -46,7 +46,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)

46 close(fd);

47 }

49 - strbuf_addf(&packname, "%s/pack/pack-", get_object_directory());

49 + strbuf_addf(&packname, "%s/pack/pack-%s.", get_object_directory(),

50 + hash_to_hex(hash));

51 finish_tmp_packfile(&packname, state->pack_tmp_name,

52 state->written, state->nr_written,

53 &state->pack_idx_opts, hash);

pack-write.c

+16 -21

@@ -462,6 +462,18 @@ struct hashfile *create_tmp_packfile(char **pack_tmp_name)

462 return hashfd(fd, *pack_tmp_name);

463 }

464

465 +static void rename_tmp_packfile(struct strbuf *name_prefix, const char *source,

466 + const char *ext)

467 +{

468 + size_t name_prefix_len = name_prefix->len;

469 +

470 + strbuf_addstr(name_prefix, ext);

471 + if (rename(source, name_prefix->buf))

472 + die_errno("unable to rename temporary file to '%s'",

473 + name_prefix->buf);

474 + strbuf_setlen(name_prefix, name_prefix_len);

475 +}

476 +

477 void finish_tmp_packfile(struct strbuf *name_buffer,

478 const char *pack_tmp_name,

479 struct pack_idx_entry **written_list,

@@ -470,7 +482,6 @@ void finish_tmp_packfile(struct strbuf *name_buffer,

482 unsigned char hash[])

483 {

484 const char *idx_tmp_name, *rev_tmp_name = NULL;

473 - int basename_len = name_buffer->len;

485

486 if (adjust_shared_perm(pack_tmp_name))

487 die_errno("unable to make temporary pack file readable");

@@ -483,26 +494,10 @@ void finish_tmp_packfile(struct strbuf *name_buffer,

494 rev_tmp_name = write_rev_file(NULL, written_list, nr_written, hash,

495 pack_idx_opts->flags);

496

486 - strbuf_addf(name_buffer, "%s.pack", hash_to_hex(hash));

487 -

488 - if (rename(pack_tmp_name, name_buffer->buf))

489 - die_errno("unable to rename temporary pack file");

490 -

491 - strbuf_setlen(name_buffer, basename_len);

492 -

493 - strbuf_addf(name_buffer, "%s.idx", hash_to_hex(hash));

494 - if (rename(idx_tmp_name, name_buffer->buf))

495 - die_errno("unable to rename temporary index file");

496 -

497 - strbuf_setlen(name_buffer, basename_len);

498 -

499 - if (rev_tmp_name) {

500 - strbuf_addf(name_buffer, "%s.rev", hash_to_hex(hash));

501 - if (rename(rev_tmp_name, name_buffer->buf))

502 - die_errno("unable to rename temporary reverse-index file");

503 - }

504 -

505 - strbuf_setlen(name_buffer, basename_len);

497 + rename_tmp_packfile(name_buffer, pack_tmp_name, "pack");

498 + rename_tmp_packfile(name_buffer, idx_tmp_name, "idx");

499 + if (rev_tmp_name)

500 + rename_tmp_packfile(name_buffer, rev_tmp_name, "rev");

501

502 free((void *)idx_tmp_name);

503 }