Re: [PATCH 3/7] merge-ort: add pool_alloc, pool_calloc, and pool_strndup wrappers

From: Jeff King <peff@peff.net>
Date: Jul 29, 2021, 15:26 UTC
Message-ID: <YQLInKdjor6Vs1yE@coredump.intra.peff.net>
In-Reply-To: <CABPp-BGjrKWXn2ZzeJ-nsZTwsDE=gbaX=1YKfiNtC70VeN9VVA@mail.gmail.com>

On Wed, Jul 28, 2021 at 04:49:18PM -0600, Elijah Newren wrote:

Show 20 quoted lines

> On Mon, Jul 26, 2021 at 8:36 AM Derrick Stolee <stolee@gmail.com> wrote:
> >
> > On 7/23/2021 8:54 AM, Elijah Newren via GitGitGadget wrote:
> > > From: Elijah Newren <newren@gmail.com>
> > >
> > > We need functions which will either call
> > >     xmalloc, xcalloc, xstrndup
> > > or
> > >     mem_pool_alloc, mem_pool_calloc, mem_pool_strndup
> > > depending on whether we have a non-NULL memory pool.  Add these
> > > functions; the next commit will make use of these.
> >
> > I briefly considered that this should just be the way the
> > mem_pool_* methods work. It does rely on the caller knowing
> > to free() the allocated memory when their pool is NULL, so
> > perhaps such a universal change might be too much. What do
> > you think?
> 
> That's interesting, but I'm worried it might be a bit much.  Do others
> on the list have an opinion here?

FWIW, I had the same thought. You can also provide a helper to make the freeing side nicer:

  static void mem_pool_free(struct mem_pool *m, void *ptr)
  {
	if (m)
		return; /* will be freed when pool frees */
	free(ptr);
  }

We do something similar with unuse_commit_buffer(), where the caller isn't aware of we pulled the buffer from cache or allocated it especially for them.

-Peff