On Wed, Sep 15, 2021 at 07:37:05PM -0700, Carlo Marcelo Arenas Belón wrote:
> b8ba412bf7 (tree-diff: avoid alloca for large allocations, 2016-06-07)
> adds a way to route some bigger allocations out of the stack and free
> them through the addition of two conveniently named macros, but leaves
> the calls to free the xalloca part, which could be also in the heap,
> if the system doesn't HAVE_ALLOCA (ex: macOS).
>
> Add the missing free call, and while at it, change the expression to
> match in both macros for easy of readability.
s/easy/ease/ or s/easy of/easier/.
> This avoids a leak reported by LSAN as while running t0000 but that
> wouldn't fail the test (which will be fixed next) :
Nit; extra space between the closing parenthesis and colon.
> diff --git a/tree-diff.c b/tree-diff.c
> index 1572615bd9..437c98a70e 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -21,7 +21,9 @@
> ALLOC_ARRAY((x), nr); \
> } while(0)
> #define FAST_ARRAY_FREE(x, nr) do { \
> - if ((nr) > 2) \
> + if ((nr) <= 2) \
> + xalloca_free((x)); \
OK. So the point is that FAST_ARRAY_ALLOC uses xalloca() for small
arrays. But that might turn into a full-blown malloc() if we don't have
alloca.h. So we need to call xalloca_free() which is a noop if we used
alloca(), but calls free() if we actually used malloc() instead.
Now that I wrote it out myself, I think you basically said as much in
the patch message. But it may have been clearer to say:
Add the missing free call, [xmalloca_free(), which is a noop if we
allocated memory in the stack frame, but a real free() if we
allocaegd in the heap instead].
Thanks,
Taylor
