Re: [PATCH] mem-pool: use st_add() in mem_pool_strvfmt()
- From
Jeff King <peff@peff.net>
- Date
- Apr 3, 2024, 21:20 UTC
- Message-ID
- <20240403212013.GA1957295@coredump.intra.peff.net>
- In-Reply-To
- <20240403204836.GC1949464@coredump.intra.peff.net>
On Wed, Apr 03, 2024 at 04:48:36PM -0400, Jeff King wrote:
Show 39 quoted lines
> Yeah, I'd prefer not to go to mkpath(), even though that's the simplest
> thing, just because we should be reducing the subtle non-reentrant parts
> of the code over time. I don't think the cleanup handling for
> mkpathdup() is too bad:
>
> diff --git a/apply.c b/apply.c
> index 432837a674..15dfe607ff 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -4502,20 +4502,25 @@ static int create_one_file(struct apply_state *state,
> unsigned int nr = getpid();
>
> for (;;) {
> - char newpath[PATH_MAX];
> - mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
> + char *newpath = mkpathdup("%s~%u", path, nr);
> res = try_create_file(state, newpath, mode, buf, size);
> - if (res < 0)
> + if (res < 0) {
> + free(newpath);
> return -1;
> + }
> if (!res) {
> - if (!rename(newpath, path))
> + if (!rename(newpath, path)) {
> + free(newpath);
> return 0;
> + }
> unlink_or_warn(newpath);
> + free(newpath);
> break;
> }
> if (errno != EEXIST)
> break;
> ++nr;
> + free(newpath);
> }
> }
> return error_errno(_("unable to write file '%s' mode %o"),OK, this misses one of the breaks, and potentially clobbers errno in that path by calling free(). So it is harder than I said. I still think it's worth doing, though.
-Peff