pack-write.c: remove unused `mtimes_name` parameter

Commit Message

Taylor Blau June 15, 2022, 12:37 a.m. UTC

`write_mtimes_file()` takes an optional parameter `mtimes_name`, which
specifies where to write the mtimes file. If it is NULL, a location is
generated with `odb_mkstemp()`.

This imitates the pattern in `write_idx_file()`, and `write_rev_file()`,
both of which have callers from the `index-pack` builtin which specify
an exact location instead of generating one.

But `write_mtimes_file()` has no such caller, and always ends up calling
odb_mkstemp(). To avoid confusion, remove the `mtimes_name` parameter
which the lone caller of `write_mtimes_file()` always fills in with
NULL.

Noticed-by: Derrick Stolee <[e-mail]>
Signed-off-by: Taylor Blau <[e-mail]>
---
 pack-write.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

Comments

Derrick Stolee June 15, 2022, 1:30 p.m. UTC | #1

On 6/14/22 8:37 PM, Taylor Blau wrote:
> `write_mtimes_file()` takes an optional parameter `mtimes_name`, which
> specifies where to write the mtimes file. If it is NULL, a location is
> generated with `odb_mkstemp()`.
> 
> This imitates the pattern in `write_idx_file()`, and `write_rev_file()`,
> both of which have callers from the `index-pack` builtin which specify
> an exact location instead of generating one.

I have a nearly-identical patch [1], but I'm happy to take Taylor's
instead. I'll plan on dropping that patch from my v2.

[1] https://lore.kernel.org/git/b67e[e-mail]/

Thanks,
-Stolee

Taylor Blau June 15, 2022, 10:55 p.m. UTC | #2

On Wed, Jun 15, 2022 at 09:30:52AM -0400, Derrick Stolee wrote:
> On 6/14/22 8:37 PM, Taylor Blau wrote:
> > `write_mtimes_file()` takes an optional parameter `mtimes_name`, which
> > specifies where to write the mtimes file. If it is NULL, a location is
> > generated with `odb_mkstemp()`.
> >
> > This imitates the pattern in `write_idx_file()`, and `write_rev_file()`,
> > both of which have callers from the `index-pack` builtin which specify
> > an exact location instead of generating one.
>
> I have a nearly-identical patch [1], but I'm happy to take Taylor's
> instead. I'll plan on dropping that patch from my v2.

;-). Great minds think alike? Either that, or one of us (me) stopped
reading the rest of this series before sending their patch.

> [1] https://lore.kernel.org/git/b67e[e-mail]/

Either version is fine with me. It's probably easier for the maintainer
to just drop my patch and take this series in one go, so I'd err on the
side of your version in [1].

Thanks,
Taylor

Patch

diff --git a/pack-write.c b/pack-write.c
index 23c0342018..6a78e0fad7 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -310,26 +310,21 @@  static void write_mtimes_trailer(struct hashfile *f, const unsigned char *hash)
 	hashwrite(f, hash, the_hash_algo->rawsz);
 }
 
-static const char *write_mtimes_file(const char *mtimes_name,
-				     struct packing_data *to_pack,
+static const char *write_mtimes_file(struct packing_data *to_pack,
 				     struct pack_idx_entry **objects,
 				     uint32_t nr_objects,
 				     const unsigned char *hash)
 {
+	struct strbuf tmp_file = STRBUF_INIT;
 	struct hashfile *f;
+	const char *mtimes_name;
 	int fd;
 
 	if (!to_pack)
 		BUG("cannot call write_mtimes_file with NULL packing_data");
 
-	if (!mtimes_name) {
-		struct strbuf tmp_file = STRBUF_INIT;
-		fd = odb_mkstemp(&tmp_file, "pack/tmp_mtimes_XXXXXX");
-		mtimes_name = strbuf_detach(&tmp_file, NULL);
-	} else {
-		unlink(mtimes_name);
-		fd = xopen(mtimes_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
-	}
+	fd = odb_mkstemp(&tmp_file, "pack/tmp_mtimes_XXXXXX");
+	mtimes_name = strbuf_detach(&tmp_file, NULL);
 	f = hashfd(fd, mtimes_name);
 
 	write_mtimes_header(f);
@@ -561,7 +556,7 @@  void stage_tmp_packfiles(struct strbuf *name_buffer,
 				      pack_idx_opts->flags);
 
 	if (pack_idx_opts->flags & WRITE_MTIMES) {
-		mtimes_tmp_name = write_mtimes_file(NULL, to_pack, written_list,
+		mtimes_tmp_name = write_mtimes_file(to_pack, written_list,
 						    nr_written,
 						    hash);
 	}