Re: [PATCH] mem-pool: use st_add() in mem_pool_strvfmt()

[Jeff King <peff@peff.net>]

On Wed, Apr 03, 2024 at 04:48:36PM -0400, Jeff King wrote:

> 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