{"thread":{"id":"58005","subject":"[PATCH] pack-write.c: remove unused `mtimes_name` parameter","startedAt":"2022-06-15T00:37:57Z","lastAt":"2022-06-15T22:55:20Z","messageCount":3,"participants":["Taylor Blau","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"457236","messageId":"f5bf68702d55b601ebd13bc4a6f1a34dc35abae5.1655253465.git.me@ttaylorr.com","threadId":"58005","inReplyTo":null,"subject":"[PATCH] pack-write.c: remove unused `mtimes_name` parameter","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-06-15T00:37:48Z","receivedAt":"2022-06-15T00:37:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"`write_mtimes_file()` takes an optional parameter `mtimes_name`, which\nspecifies where to write the mtimes file. If it is NULL, a location is\ngenerated with `odb_mkstemp()`.\n\nThis imitates the pattern in `write_idx_file()`, and `write_rev_file()`,\nboth of which have callers from the `index-pack` builtin which specify\nan exact location instead of generating one.\n\nBut `write_mtimes_file()` has no such caller, and always ends up calling\nodb_mkstemp(). To avoid confusion, remove the `mtimes_name` parameter\nwhich the lone caller of `write_mtimes_file()` always fills in with\nNULL.\n\nNoticed-by: Derrick Stolee <derrickstolee@github.com>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n pack-write.c | 17 ++++++-----------\n 1 file changed, 6 insertions(+), 11 deletions(-)\n\ndiff --git a/pack-write.c b/pack-write.c\nindex 23c0342018..6a78e0fad7 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -310,26 +310,21 @@ static void write_mtimes_trailer(struct hashfile *f, const unsigned char *hash)\n \thashwrite(f, hash, the_hash_algo->rawsz);\n }\n \n-static const char *write_mtimes_file(const char *mtimes_name,\n-\t\t\t\t     struct packing_data *to_pack,\n+static const char *write_mtimes_file(struct packing_data *to_pack,\n \t\t\t\t     struct pack_idx_entry **objects,\n \t\t\t\t     uint32_t nr_objects,\n \t\t\t\t     const unsigned char *hash)\n {\n+\tstruct strbuf tmp_file = STRBUF_INIT;\n \tstruct hashfile *f;\n+\tconst char *mtimes_name;\n \tint fd;\n \n \tif (!to_pack)\n \t\tBUG(\"cannot call write_mtimes_file with NULL packing_data\");\n \n-\tif (!mtimes_name) {\n-\t\tstruct strbuf tmp_file = STRBUF_INIT;\n-\t\tfd = odb_mkstemp(&tmp_file, \"pack/tmp_mtimes_XXXXXX\");\n-\t\tmtimes_name = strbuf_detach(&tmp_file, NULL);\n-\t} else {\n-\t\tunlink(mtimes_name);\n-\t\tfd = xopen(mtimes_name, O_CREAT|O_EXCL|O_WRONLY, 0600);\n-\t}\n+\tfd = odb_mkstemp(&tmp_file, \"pack/tmp_mtimes_XXXXXX\");\n+\tmtimes_name = strbuf_detach(&tmp_file, NULL);\n \tf = hashfd(fd, mtimes_name);\n \n \twrite_mtimes_header(f);\n@@ -561,7 +556,7 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,\n \t\t\t\t      pack_idx_opts->flags);\n \n \tif (pack_idx_opts->flags & WRITE_MTIMES) {\n-\t\tmtimes_tmp_name = write_mtimes_file(NULL, to_pack, written_list,\n+\t\tmtimes_tmp_name = write_mtimes_file(to_pack, written_list,\n \t\t\t\t\t\t    nr_written,\n \t\t\t\t\t\t    hash);\n \t}\n-- \n2.35.1\n"},{"id":"457280","messageId":"a7c81b77-651d-3374-6db8-f46ad01a8319@github.com","threadId":"58005","inReplyTo":"f5bf68702d55b601ebd13bc4a6f1a34dc35abae5.1655253465.git.me@ttaylorr.com","subject":"Re: [PATCH] pack-write.c: remove unused `mtimes_name` parameter","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-15T13:30:52Z","receivedAt":"2022-06-15T13:30:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/14/22 8:37 PM, Taylor Blau wrote:\n> `write_mtimes_file()` takes an optional parameter `mtimes_name`, which\n> specifies where to write the mtimes file. If it is NULL, a location is\n> generated with `odb_mkstemp()`.\n> \n> This imitates the pattern in `write_idx_file()`, and `write_rev_file()`,\n> both of which have callers from the `index-pack` builtin which specify\n> an exact location instead of generating one.\n\nI have a nearly-identical patch [1], but I'm happy to take Taylor's\ninstead. I'll plan on dropping that patch from my v2.\n\n[1] https://lore.kernel.org/git/b67e110bf60e820874de94c64ee8c32d69413877.1655242070.git.gitgitgadget@gmail.com/\n\nThanks,\n-Stolee\n"},{"id":"457324","messageId":"YqpjUiT45Oy8a577@nand.local","threadId":"58005","inReplyTo":"a7c81b77-651d-3374-6db8-f46ad01a8319@github.com","subject":"Re: [PATCH] pack-write.c: remove unused `mtimes_name` parameter","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-06-15T22:55:14Z","receivedAt":"2022-06-15T22:55:20Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jun 15, 2022 at 09:30:52AM -0400, Derrick Stolee wrote:\n> On 6/14/22 8:37 PM, Taylor Blau wrote:\n> > `write_mtimes_file()` takes an optional parameter `mtimes_name`, which\n> > specifies where to write the mtimes file. If it is NULL, a location is\n> > generated with `odb_mkstemp()`.\n> >\n> > This imitates the pattern in `write_idx_file()`, and `write_rev_file()`,\n> > both of which have callers from the `index-pack` builtin which specify\n> > an exact location instead of generating one.\n>\n> I have a nearly-identical patch [1], but I'm happy to take Taylor's\n> instead. I'll plan on dropping that patch from my v2.\n\n;-). Great minds think alike? Either that, or one of us (me) stopped\nreading the rest of this series before sending their patch.\n\n> [1] https://lore.kernel.org/git/b67e110bf60e820874de94c64ee8c32d69413877.1655242070.git.gitgitgadget@gmail.com/\n\nEither version is fine with me. It's probably easier for the maintainer\nto just drop my patch and take this series in one go, so I'd err on the\nside of your version in [1].\n\nThanks,\nTaylor\n"}]}