Re: [PATCH] reftable: use xmalloc() and xrealloc()
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Apr 8, 2024, 05:44 UTC
- Message-ID
- <ZhOETox9FTIOAShN@tanuki>
- In-Reply-To
- <963961ee-0f1d-42b8-8dda-5838e7a2ed94@web.de>
On Sat, Apr 06, 2024 at 10:37:55PM +0200, René Scharfe wrote:
Show 32 quoted lines
> malloc(3) and realloc(3) can fail and return NULL. None of the reftable
> code checks for that possibility and would happily dereference NULL
> pointers. Use xmalloc() and xrealloc() instead like in the rest of Git
> to report allocation errors and exit cleanly, and to also honor the
> environment variable GIT_ALLOC_LIMIT.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> reftable/publicbasics.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/reftable/publicbasics.c b/reftable/publicbasics.c
> index 44b84a125e..f33a65df34 100644
> --- a/reftable/publicbasics.c
> +++ b/reftable/publicbasics.c
> @@ -19,14 +19,14 @@ void *reftable_malloc(size_t sz)
> {
> if (reftable_malloc_ptr)
> return (*reftable_malloc_ptr)(sz);
> - return malloc(sz);
> + return xmalloc(sz);
> }
>
> void *reftable_realloc(void *p, size_t sz)
> {
> if (reftable_realloc_ptr)
> return (*reftable_realloc_ptr)(p, sz);
> - return realloc(p, sz);
> + return xrealloc(p, sz);
> }
>
> void reftable_free(void *p)These are part of the library interfaces that should ideally not be tied to the Git code base at all so that they can theoretically be reused by another project like libgit2. So I think that instead of rewriting the generic fallbacks we should call `reftable_set_alloc()` somewhen early in Git's startup code.
It does raise the question what to do about the generic fallbacks. We could start calling `abort()` when we observe allocation failures. It's not exactly nice behaviour in a library though, where the caller may in fact want to handle this case. But it may at least be better than failing on a `NULL` pointer exception somewhere down the road. So it might be the best alternative for now. We could then conver the reftable library over time to handle allocation failures and, once that's done, we can eventually drop such a call to `abort()`.
Cc'ing Han-Wen's new mail address as he no longer works at Google.
Patrick