git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Taylor Blau" <me@ttaylorr.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 1/3] pack-write: use more idiomatic strbuf usage for packname construction
Date: Tue,  7 Sep 2021 21:42:36 +0200	[thread overview]
Message-ID: <patch-1.3-0e6ef07ce00-20210907T193600Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-0.3-00000000000-20210907T193600Z-avarab@gmail.com>

Change code added in 5889271114a (finish_tmp_packfile():use strbuf for
pathname construction, 2014-03-03) to do strbuf_reset() instead of
noting the length of the base template, and doing a strbuf_setlen() to
reset it, also change the spacing in the finish_tmp_packfile() so that
each setup of the template, rename, and strbuf_reset() is grouped
together.

Since the prototype of the previous "name_buffer" now has a "const"
use this chance to wrap the overly long definition of the
finish_tmp_packfile() function.

This doesn't really matter for now, but as we'll see makes the
subsequent change much easier, as we won't need to juggle the basename
template v.s. its current contents anymore when writing bitmaps.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/pack-objects.c | 17 +++++++++++------
 pack-write.c           | 27 ++++++++++++---------------
 pack.h                 |  7 ++++++-
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index df49f656b96..717003563db 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1216,7 +1216,7 @@ static void write_pack_file(void)
 
 		if (!pack_to_stdout) {
 			struct stat st;
-			struct strbuf tmpname = STRBUF_INIT;
+			struct strbuf tmp_basename = STRBUF_INIT;
 
 			/*
 			 * Packs are runtime accessed in their mtime
@@ -1237,7 +1237,7 @@ static void write_pack_file(void)
 					warning_errno(_("failed utime() on %s"), pack_tmp_name);
 			}
 
-			strbuf_addf(&tmpname, "%s-", base_name);
+			strbuf_addf(&tmp_basename, "%s-", base_name);
 
 			if (write_bitmap_index) {
 				bitmap_writer_set_checksum(hash);
@@ -1245,12 +1245,16 @@ static void write_pack_file(void)
 					&to_pack, written_list, nr_written);
 			}
 
-			finish_tmp_packfile(&tmpname, pack_tmp_name,
+			finish_tmp_packfile(&tmp_basename, pack_tmp_name,
 					    written_list, nr_written,
 					    &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",
+					    tmp_basename.buf,
+					    hash_to_hex(hash));
 
 				stop_progress(&progress_state);
 
@@ -1258,11 +1262,12 @@ 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);
+			strbuf_release(&tmp_basename);
 			free(pack_tmp_name);
 			puts(hash_to_hex(hash));
 		}
diff --git a/pack-write.c b/pack-write.c
index 277c60165e8..57b9fc11423 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -462,15 +462,15 @@ struct hashfile *create_tmp_packfile(char **pack_tmp_name)
 	return hashfd(fd, *pack_tmp_name);
 }
 
-void finish_tmp_packfile(struct strbuf *name_buffer,
+void finish_tmp_packfile(const struct strbuf *tmp_basename,
 			 const char *pack_tmp_name,
 			 struct pack_idx_entry **written_list,
 			 uint32_t nr_written,
 			 struct pack_idx_option *pack_idx_opts,
 			 unsigned char hash[])
 {
+	struct strbuf sb = STRBUF_INIT;
 	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 +483,23 @@ 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))
+	strbuf_addf(&sb, "%s%s.pack", tmp_basename->buf, hash_to_hex(hash));
+	if (rename(pack_tmp_name, sb.buf))
 		die_errno("unable to rename temporary pack file");
-
-	strbuf_setlen(name_buffer, basename_len);
+	strbuf_reset(&sb);
 
 	if (rev_tmp_name) {
-		strbuf_addf(name_buffer, "%s.rev", hash_to_hex(hash));
-		if (rename(rev_tmp_name, name_buffer->buf))
+		strbuf_addf(&sb, "%s%s.rev", tmp_basename->buf,
+			    hash_to_hex(hash));
+		if (rename(rev_tmp_name, sb.buf))
 			die_errno("unable to rename temporary reverse-index file");
-
-		strbuf_setlen(name_buffer, basename_len);
+		strbuf_reset(&sb);
 	}
 
-	strbuf_addf(name_buffer, "%s.idx", hash_to_hex(hash));
-	if (rename(idx_tmp_name, name_buffer->buf))
+	strbuf_addf(&sb, "%s%s.idx", tmp_basename->buf, hash_to_hex(hash));
+	if (rename(idx_tmp_name, sb.buf))
 		die_errno("unable to rename temporary index file");
-
-	strbuf_setlen(name_buffer, basename_len);
+	strbuf_reset(&sb);
 
 	free((void *)idx_tmp_name);
 }
diff --git a/pack.h b/pack.h
index fa139545262..ae0c9e04cd9 100644
--- a/pack.h
+++ b/pack.h
@@ -110,6 +110,11 @@ 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, const char *pack_tmp_name, struct pack_idx_entry **written_list, uint32_t nr_written, struct pack_idx_option *pack_idx_opts, unsigned char sha1[]);
+void finish_tmp_packfile(const struct strbuf *name_buffer,
+			 const char *pack_tmp_name,
+			 struct pack_idx_entry **written_list,
+			 uint32_t nr_written,
+			 struct pack_idx_option *pack_idx_opts,
+			 unsigned char sha1[]);
 
 #endif
-- 
2.33.0.818.gd2ef2916285


  reply	other threads:[~2021-09-07 19:42 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01  2:05 [PATCH 0/2] pack-write,repack: prevent opening packs too early Taylor Blau
2021-09-01  2:06 ` [PATCH 1/2] pack-write.c: rename `.idx` file into place last Taylor Blau
2021-09-01  2:06 ` [PATCH 2/2] builtin/repack.c: move `.idx` files " Taylor Blau
2021-09-01  3:53 ` [PATCH 0/2] pack-write,repack: prevent opening packs too early Jeff King
2021-09-01  4:29   ` Taylor Blau
2021-09-01  4:59     ` Jeff King
2021-09-01  5:12       ` Taylor Blau
2021-09-01  6:08         ` Jeff King
2021-09-01 21:40           ` Taylor Blau
2021-09-07 16:07             ` Jeff King
2021-09-07 19:42 ` [PATCH 0/3] rename *.idx file into place last (also after *.bitmap) Ævar Arnfjörð Bjarmason
2021-09-07 19:42   ` Ævar Arnfjörð Bjarmason [this message]
2021-09-07 22:21     ` [PATCH 1/3] pack-write: use more idiomatic strbuf usage for packname construction Taylor Blau
2021-09-07 23:22       ` Ævar Arnfjörð Bjarmason
2021-09-07 19:42   ` [PATCH 2/3] pack-write: split up finish_tmp_packfile() function Ævar Arnfjörð Bjarmason
2021-09-07 22:28     ` Taylor Blau
2021-09-07 19:42   ` [PATCH 3/3] pack-write: rename *.idx file into place last (really!) Ævar Arnfjörð Bjarmason
2021-09-07 22:31     ` Taylor Blau
2021-09-07 22:36   ` [PATCH 0/3] rename *.idx file into place last (also after *.bitmap) Taylor Blau
2021-09-07 19:48 ` [PATCH 0/2] pack-write,repack: prevent opening packs too early Ævar Arnfjörð Bjarmason
2021-09-08  0:38 ` [PATCH v2 0/4] rename *.idx file into place last (also after *.bitmap) Ævar Arnfjörð Bjarmason
2021-09-08  0:38   ` [PATCH v2 1/4] pack.h: line-wrap the definition of finish_tmp_packfile() Ævar Arnfjörð Bjarmason
2021-09-08  0:38   ` [PATCH v2 2/4] pack-write: refactor renaming in finish_tmp_packfile() Ævar Arnfjörð Bjarmason
2021-09-08  4:22     ` Taylor Blau
2021-09-08  0:38   ` [PATCH v2 3/4] pack-write: split up finish_tmp_packfile() function Ævar Arnfjörð Bjarmason
2021-09-08  0:38   ` [PATCH v2 4/4] pack-write: rename *.idx file into place last (really!) Ævar Arnfjörð Bjarmason
2021-09-08  1:14     ` Ævar Arnfjörð Bjarmason
2021-09-08  9:18       ` Ævar Arnfjörð Bjarmason
2021-09-08  4:24     ` Taylor Blau
2021-09-08 22:17 ` [PATCH v2 0/3] prevent opening packs too early Taylor Blau
2021-09-08 22:17   ` [PATCH v2 1/3] pack-write.c: rename `.idx` files into place last Taylor Blau
2021-09-08 22:17   ` [PATCH v2 2/3] builtin/repack.c: move " Taylor Blau
2021-09-08 22:17   ` [PATCH v2 3/3] builtin/index-pack.c: " Taylor Blau
2021-09-08 23:52   ` [PATCH v2 0/3] prevent opening packs too early Ævar Arnfjörð Bjarmason
2021-09-09  0:50     ` Ævar Arnfjörð Bjarmason
2021-09-09  1:13       ` Taylor Blau
2021-09-09  1:33         ` Ævar Arnfjörð Bjarmason
2021-09-09  2:36           ` Ævar Arnfjörð Bjarmason
2021-09-09  2:49             ` Taylor Blau
2021-09-09  3:24 ` [PATCH 0/9] packfile: avoid .idx rename races Taylor Blau
2021-09-09  3:24   ` [PATCH 1/9] pack.h: line-wrap the definition of finish_tmp_packfile() Taylor Blau
2021-09-09  3:24   ` [PATCH 2/9] bulk-checkin.c: store checksum directly Taylor Blau
2021-09-09  7:38     ` Ævar Arnfjörð Bjarmason
2021-09-09  3:24   ` [PATCH 3/9] pack-write: refactor renaming in finish_tmp_packfile() Taylor Blau
2021-09-09 19:29     ` Junio C Hamano
2021-09-09 21:07       ` Taylor Blau
2021-09-09 23:30         ` Junio C Hamano
2021-09-09 23:31           ` Taylor Blau
2021-09-10  1:29         ` Junio C Hamano
2021-09-09  3:24   ` [PATCH 4/9] pack-write.c: rename `.idx` files after `*.rev` Taylor Blau
2021-09-09  7:46     ` Ævar Arnfjörð Bjarmason
2021-09-09 14:37       ` Taylor Blau
2021-09-09 19:32     ` Junio C Hamano
2021-09-09  3:25   ` [PATCH 5/9] builtin/repack.c: move `.idx` files into place last Taylor Blau
2021-09-09 19:38     ` Junio C Hamano
2021-09-09 21:08       ` Taylor Blau
2021-09-09  3:25   ` [PATCH 6/9] index-pack: refactor renaming in final() Taylor Blau
2021-09-09 19:45     ` Junio C Hamano
2021-09-09 21:11       ` Taylor Blau
2021-09-09  3:25   ` [PATCH 7/9] builtin/index-pack.c: move `.idx` files into place last Taylor Blau
2021-09-09  7:52     ` Ævar Arnfjörð Bjarmason
2021-09-09 19:45     ` Junio C Hamano
2021-09-09  3:25   ` [PATCH 8/9] pack-write: split up finish_tmp_packfile() function Taylor Blau
2021-09-09  3:25   ` [PATCH 9/9] pack-objects: rename .idx files into place after .bitmap files Taylor Blau
2021-09-09  7:54     ` Ævar Arnfjörð Bjarmason
2021-09-09 19:52       ` Junio C Hamano
2021-09-09 21:13         ` Taylor Blau
2021-09-09  8:06   ` [PATCH 0/9] packfile: avoid .idx rename races Ævar Arnfjörð Bjarmason
2021-09-09 14:40     ` Taylor Blau
2021-09-09 19:52       ` Junio C Hamano
2021-09-09 23:24   ` [PATCH v2 " Taylor Blau
2021-09-09 23:24     ` [PATCH v2 1/9] pack.h: line-wrap the definition of finish_tmp_packfile() Taylor Blau
2021-09-09 23:24     ` [PATCH v2 2/9] bulk-checkin.c: store checksum directly Taylor Blau
2021-09-09 23:24     ` [PATCH v2 3/9] pack-write: refactor renaming in finish_tmp_packfile() Taylor Blau
2021-09-09 23:24     ` [PATCH v2 4/9] pack-write.c: rename `.idx` files after `*.rev` Taylor Blau
2021-09-09 23:24     ` [PATCH v2 5/9] builtin/repack.c: move `.idx` files into place last Taylor Blau
2021-09-09 23:24     ` [PATCH v2 6/9] index-pack: refactor renaming in final() Taylor Blau
2021-09-09 23:24     ` [PATCH v2 7/9] builtin/index-pack.c: move `.idx` files into place last Taylor Blau
2021-09-09 23:24     ` [PATCH v2 8/9] pack-write: split up finish_tmp_packfile() function Taylor Blau
2021-09-09 23:25     ` [PATCH v2 9/9] pack-objects: rename .idx files into place after .bitmap files Taylor Blau
2021-09-10  1:35     ` [PATCH v2 0/9] packfile: avoid .idx rename races Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-1.3-0e6ef07ce00-20210907T193600Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).