Re: [PATCH 1/2] mem-pool: add mem_pool_strfmt()

[Jeff King <peff@peff.net>]

On Sun, Feb 25, 2024 at 12:39:44PM +0100, René Scharfe wrote:

> +static char *mem_pool_strvfmt(struct mem_pool *pool, const char *fmt,
> +			      va_list ap)
> +{
> +	struct mp_block *block = pool->mp_block;
> +	char *next_free = block ? block->next_free : NULL;
> +	size_t available = block ? block->end - block->next_free : 0;
> +	va_list cp;
> +	int len, len2;
> +	char *ret;
> +
> +	va_copy(cp, ap);
> +	len = vsnprintf(next_free, available, fmt, cp);
> +	va_end(cp);
> +	if (len < 0)
> +		BUG("your vsnprintf is broken (returned %d)", len);
> +
> +	ret = mem_pool_alloc(pool, len + 1);  /* 1 for NUL */
> +
> +	/* Shortcut; relies on mem_pool_alloc() not touching buffer contents. */
> +	if (ret == next_free)
> +		return ret;
> +
> +	len2 = vsnprintf(ret, len + 1, fmt, ap);
> +	if (len2 != len)
> +		BUG("your vsnprintf is broken (returns inconsistent lengths)");
> +	return ret;
> +}

This is pulling heavily from strbuf_vaddf(). This might be a dumb idea,
but... would it be reasonable to instead push a global flag that causes
xmalloc() to use a memory pool instead of the regular heap?

Then you could do something like:

  push_mem_pool(pool);
  str = xstrfmt("%.*s~%d^%d", ...etc...);
  pop_mem_pool(pool);

It's a little more involved at the caller, but it means that it now
works for all allocations, not just this one string helper.

Obviously you'd want it to be a thread-local value to prevent races. But
I still wonder if it could cause havoc when some sub-function makes an
allocation that the caller does not expect.

-Peff