git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Jeff KingNext: René Scharfe
Message 7 of 9 in “mem-pool: use st_add() in mem_pool_strvfmt()”
  1. mem-pool: use st_add() in mem_pool_strvfmt()René Scharfe, Mar 31, 2024
  2. Jeff KingApr 1, 2024
  3. René ScharfeApr 2, 2024
  4. Jeff KingApr 3, 2024
  5. René ScharfeApr 3, 2024
  6. Jeff KingApr 3, 2024
  7. Jeff KingApr 3, 2024
  8. René ScharfeApr 5, 2024
  9. Jeff KingApr 5, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.