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

[PATCH v2 2/4] pack-write: refactor renaming in finish_tmp_packfile()

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 8, 2021, 00:38 UTC
Message-ID
<patch-v2-2.4-7b39f4599b-20210908T003631Z-avarab@gmail.com>
In-Reply-To
<cover-v2-0.4-0000000000-20210908T003631Z-avarab@gmail.com>

Refactor the renaming in finish_tmp_packfile() so that it takes a "const struct strbuf *" instead of a non-const, and refactor the repetitive renaming pattern in finish_tmp_packfile() to use a new static rename_tmp_packfile() helper function.

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).

Since the memory allocation is not a bottleneck here we can afford a bit more readability at the cost of re-allocating this new "struct strbuf sb".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/pack-objects.c |  8 ++++++--
 pack-write.c           | 41 +++++++++++++++++++----------------------
 pack.h                 |  2 +-
 3 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index df49f656b9..f2569b5ca2 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1250,7 +1250,10 @@ static void write_pack_file(void)
 					    &pack_idx_opts, hash);
 
 			if (write_bitmap_index) {
-				strbuf_addf(&tmpname, "%s.bitmap", hash_to_hex(hash));
+				struct strbuf sb = STRBUF_INIT;
+
+				strbuf_addf(&sb, "%s%s.bitmap", tmpname.buf,
+					    hash_to_hex(hash));
 
 				stop_progress(&progress_state);
 
@@ -1258,8 +1261,9 @@ static void write_pack_file(void)
 				bitmap_writer_select_commits(indexed_commits, indexed_commits_nr, -1);
 				bitmap_writer_build(&to_pack);
 				bitmap_writer_finish(written_list, nr_written,
-						     tmpname.buf, write_bitmap_options);
+						     sb.buf, write_bitmap_options);
 				write_bitmap_index = 0;
+				strbuf_release(&sb);
 			}
 
 			strbuf_release(&tmpname);
diff --git a/pack-write.c b/pack-write.c
index 277c60165e..363b395d67 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -462,7 +462,21 @@ struct hashfile *create_tmp_packfile(char **pack_tmp_name)
 	return hashfd(fd, *pack_tmp_name);
 }
 
-void finish_tmp_packfile(struct strbuf *name_buffer,
+static void rename_tmp_packfile(const char *source,
+				 const struct strbuf *basename,
+				 const unsigned char hash[],
+				 const char *ext)
+{
+	struct strbuf sb = STRBUF_INIT;
+
+	strbuf_addf(&sb, "%s%s.%s", basename->buf, hash_to_hex(hash), ext);
+	if (rename(source, sb.buf))
+		die_errno("unable to rename temporary '*.%s' file to '%s'",
+			  ext, sb.buf);
+	strbuf_release(&sb);
+}
+
+void finish_tmp_packfile(const struct strbuf *basename,
 			 const char *pack_tmp_name,
 			 struct pack_idx_entry **written_list,
 			 uint32_t nr_written,
@@ -470,7 +484,6 @@ void finish_tmp_packfile(struct strbuf *name_buffer,
 			 unsigned char hash[])
 {
 	const char *idx_tmp_name, *rev_tmp_name = NULL;
-	int basename_len = name_buffer->len;
 
 	if (adjust_shared_perm(pack_tmp_name))
 		die_errno("unable to make temporary pack file readable");
@@ -483,26 +496,10 @@ void finish_tmp_packfile(struct strbuf *name_buffer,
 	rev_tmp_name = write_rev_file(NULL, written_list, nr_written, hash,
 				      pack_idx_opts->flags);
 
-	strbuf_addf(name_buffer, "%s.pack", hash_to_hex(hash));
-
-	if (rename(pack_tmp_name, name_buffer->buf))
-		die_errno("unable to rename temporary pack file");
-
-	strbuf_setlen(name_buffer, basename_len);
-
-	if (rev_tmp_name) {
-		strbuf_addf(name_buffer, "%s.rev", hash_to_hex(hash));
-		if (rename(rev_tmp_name, name_buffer->buf))
-			die_errno("unable to rename temporary reverse-index file");
-
-		strbuf_setlen(name_buffer, basename_len);
-	}
-
-	strbuf_addf(name_buffer, "%s.idx", hash_to_hex(hash));
-	if (rename(idx_tmp_name, name_buffer->buf))
-		die_errno("unable to rename temporary index file");
-
-	strbuf_setlen(name_buffer, basename_len);
+	rename_tmp_packfile(pack_tmp_name, basename, hash, "pack");
+	if (rev_tmp_name)
+		rename_tmp_packfile(rev_tmp_name, basename, hash, "rev");
+	rename_tmp_packfile(idx_tmp_name, basename, hash, "idx");
 
 	free((void *)idx_tmp_name);
 }
diff --git a/pack.h b/pack.h
index 1c17254c0a..594d5176aa 100644
--- a/pack.h
+++ b/pack.h
@@ -110,7 +110,7 @@ int encode_in_pack_object_header(unsigned char *hdr, int hdr_len,
 int read_pack_header(int fd, struct pack_header *);
 
 struct hashfile *create_tmp_packfile(char **pack_tmp_name);
-void finish_tmp_packfile(struct strbuf *name_buffer,
+void finish_tmp_packfile(const struct strbuf *basename,
 			 const char *pack_tmp_name,
 			 struct pack_idx_entry **written_list,
 			 uint32_t nr_written,
-- 
2.33.0.819.gea1b153a43c
Previous: Ævar Arnfjörð BjarmasonNext: Taylor Blau
Message 23 of 81 in “pack-write,repack: prevent opening packs too early”
  1. 0/2 pack-write,repack: prevent opening packs too earlyTaylor Blau, Sep 1, 2021
  2. 1/2 pack-write.c: rename `.idx` file into place lastTaylor Blau, Sep 1, 2021
  3. 2/2 builtin/repack.c: move `.idx` files into place lastTaylor Blau, Sep 1, 2021
  4. Jeff KingSep 1, 2021
  5. Taylor BlauSep 1, 2021
  6. Jeff KingSep 1, 2021
  7. Taylor BlauSep 1, 2021
  8. Jeff KingSep 1, 2021
  9. Taylor BlauSep 1, 2021
  10. Jeff KingSep 7, 2021
  11. 0/3 rename *.idx file into place last (also after *.bitmap)Ævar Arnfjörð Bjarmason, Sep 7, 2021
  12. 1/3 pack-write: use more idiomatic strbuf usage for packname constructionÆvar Arnfjörð Bjarmason, Sep 7, 2021
  13. Taylor BlauSep 7, 2021
  14. Ævar Arnfjörð BjarmasonSep 7, 2021
  15. 2/3 pack-write: split up finish_tmp_packfile() functionÆvar Arnfjörð Bjarmason, Sep 7, 2021
  16. Taylor BlauSep 7, 2021
  17. 3/3 pack-write: rename *.idx file into place last (really!)Ævar Arnfjörð Bjarmason, Sep 7, 2021
  18. Taylor BlauSep 7, 2021
  19. Taylor BlauSep 7, 2021
  20. Ævar Arnfjörð BjarmasonSep 7, 2021
  21. 0/4 rename *.idx file into place last (also after *.bitmap)Ævar Arnfjörð Bjarmason, Sep 8, 2021
  22. 1/4 pack.h: line-wrap the definition of finish_tmp_packfile()Ævar Arnfjörð Bjarmason, Sep 8, 2021
  23. 2/4 pack-write: refactor renaming in finish_tmp_packfile()Ævar Arnfjörð Bjarmason, Sep 8, 2021
  24. Taylor BlauSep 8, 2021
  25. 3/4 pack-write: split up finish_tmp_packfile() functionÆvar Arnfjörð Bjarmason, Sep 8, 2021
  26. 4/4 pack-write: rename *.idx file into place last (really!)Ævar Arnfjörð Bjarmason, Sep 8, 2021
  27. Ævar Arnfjörð BjarmasonSep 8, 2021
  28. Ævar Arnfjörð BjarmasonSep 8, 2021
  29. Taylor BlauSep 8, 2021
  30. 0/3 prevent opening packs too earlyTaylor Blau, Sep 8, 2021
  31. 1/3 pack-write.c: rename `.idx` files into place lastTaylor Blau, Sep 8, 2021
  32. 2/3 builtin/repack.c: move `.idx` files into place lastTaylor Blau, Sep 8, 2021
  33. 3/3 builtin/index-pack.c: move `.idx` files into place lastTaylor Blau, Sep 8, 2021
  34. Ævar Arnfjörð BjarmasonSep 8, 2021
  35. Ævar Arnfjörð BjarmasonSep 9, 2021
  36. Taylor BlauSep 9, 2021
  37. Ævar Arnfjörð BjarmasonSep 9, 2021
  38. Ævar Arnfjörð BjarmasonSep 9, 2021
  39. Taylor BlauSep 9, 2021
  40. 0/9 packfile: avoid .idx rename racesTaylor Blau, Sep 9, 2021
  41. 1/9 pack.h: line-wrap the definition of finish_tmp_packfile()Taylor Blau, Sep 9, 2021
  42. 2/9 bulk-checkin.c: store checksum directlyTaylor Blau, Sep 9, 2021
  43. Ævar Arnfjörð BjarmasonSep 9, 2021
  44. 3/9 pack-write: refactor renaming in finish_tmp_packfile()Taylor Blau, Sep 9, 2021
  45. Junio C HamanoSep 9, 2021
  46. Taylor BlauSep 9, 2021
  47. Junio C HamanoSep 9, 2021
  48. Taylor BlauSep 9, 2021
  49. Junio C HamanoSep 10, 2021
  50. 4/9 pack-write.c: rename `.idx` files after `*.rev`Taylor Blau, Sep 9, 2021
  51. Ævar Arnfjörð BjarmasonSep 9, 2021
  52. Taylor BlauSep 9, 2021
  53. Junio C HamanoSep 9, 2021
  54. 5/9 builtin/repack.c: move `.idx` files into place lastTaylor Blau, Sep 9, 2021
  55. Junio C HamanoSep 9, 2021
  56. Taylor BlauSep 9, 2021
  57. 6/9 index-pack: refactor renaming in final()Taylor Blau, Sep 9, 2021
  58. Junio C HamanoSep 9, 2021
  59. Taylor BlauSep 9, 2021
  60. 8/9 pack-write: split up finish_tmp_packfile() functionTaylor Blau, Sep 9, 2021
  61. 7/9 builtin/index-pack.c: move `.idx` files into place lastTaylor Blau, Sep 9, 2021
  62. Ævar Arnfjörð BjarmasonSep 9, 2021
  63. Junio C HamanoSep 9, 2021
  64. 9/9 pack-objects: rename .idx files into place after .bitmap filesTaylor Blau, Sep 9, 2021
  65. Ævar Arnfjörð BjarmasonSep 9, 2021
  66. Junio C HamanoSep 9, 2021
  67. Taylor BlauSep 9, 2021
  68. Ævar Arnfjörð BjarmasonSep 9, 2021
  69. Taylor BlauSep 9, 2021
  70. Junio C HamanoSep 9, 2021
  71. 0/9 packfile: avoid .idx rename racesTaylor Blau, Sep 9, 2021
  72. 1/9 pack.h: line-wrap the definition of finish_tmp_packfile()Taylor Blau, Sep 9, 2021
  73. 2/9 bulk-checkin.c: store checksum directlyTaylor Blau, Sep 9, 2021
  74. 3/9 pack-write: refactor renaming in finish_tmp_packfile()Taylor Blau, Sep 9, 2021
  75. 4/9 pack-write.c: rename `.idx` files after `*.rev`Taylor Blau, Sep 9, 2021
  76. 5/9 builtin/repack.c: move `.idx` files into place lastTaylor Blau, Sep 9, 2021
  77. 6/9 index-pack: refactor renaming in final()Taylor Blau, Sep 9, 2021
  78. 8/9 pack-write: split up finish_tmp_packfile() functionTaylor Blau, Sep 9, 2021
  79. 7/9 builtin/index-pack.c: move `.idx` files into place lastTaylor Blau, Sep 9, 2021
  80. 9/9 pack-objects: rename .idx files into place after .bitmap filesTaylor Blau, Sep 9, 2021
  81. Junio C HamanoSep 10, 2021

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.