On Tue, Apr 18, 2023 at 06:30:05AM -0400, Jeff King wrote: > On Mon, Apr 17, 2023 at 04:54:15PM -0400, Taylor Blau wrote: > > > The function `stage_tmp_packfiles()` generates a filename to use for > > staging the contents of what will become the pack's ".mtimes" file. > > > > The name is generated in `write_mtimes_file()` and the result is > > returned back to `stage_tmp_packfiles()` which uses it to rename the > > temporary file into place via `rename_tmp_packfiles()`. > > > > `write_mtimes_file()` returns a `const char *`, indicating that callers > > are not expected to free its result (similar to, e.g., `oid_to_hex()`). > > But callers are expected to free its result, so this return type is > > incorrect. > > Makes sense. I do think in the long run that it might make sense for all > of these pack-write tmpfiles to tempfile.[ch] (either directly, or via > register_tempfile() after creating the file). And then it would be safe > to pass around the tempfile struct itself, which contains the filename, > without worrying so much about ownership issues. > > But that's a much bigger change that shouldn't be part of your series. Agreed on both fronts. I'll put investigating something like this on my list of things to-do. Thanks, Taylor
