# [PATCH] pack-write.c: remove unused `mtimes_name` parameter 3 messages from 2022-06-15 to 2022-06-15. Participants: Taylor Blau, Derrick Stolee. Thread: https://gitlist.dev/t/58005 ## Taylor Blau, 2022-06-15 00:37 Subject: [PATCH] pack-write.c: remove unused `mtimes_name` parameter Message-ID: URL: https://gitlist.dev/e/f5bf68702d55b601ebd13bc4a6f1a34dc35abae5.1655253465.git.me%40ttaylorr.com ``` `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 Signed-off-by: Taylor Blau --- pack-write.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) 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); } -- 2.35.1 ``` ## Derrick Stolee, 2022-06-15 13:30 Subject: Re: [PATCH] pack-write.c: remove unused `mtimes_name` parameter Message-ID: URL: https://gitlist.dev/e/a7c81b77-651d-3374-6db8-f46ad01a8319%40github.com In-Reply-To: ``` 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/b67e110bf60e820874de94c64ee8c32d69413877.1655242070.git.gitgitgadget@gmail.com/ Thanks, -Stolee ``` ## Taylor Blau, 2022-06-15 22:55 Subject: Re: [PATCH] pack-write.c: remove unused `mtimes_name` parameter Message-ID: URL: https://gitlist.dev/e/YqpjUiT45Oy8a577%40nand.local In-Reply-To: ``` 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/b67e110bf60e820874de94c64ee8c32d69413877.1655242070.git.gitgitgadget@gmail.com/ 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 ```