t-reftable-basics: allow for `malloc` to be `#define`d

From: Johannes Schindelin <[email protected]>

As indicated by the #undef malloc line in reftable/basics.h, it is quite common to use allocators other than the default one by defining malloc constants and friends.

This pattern is used e.g. in Git for Windows, which uses the powerful and performant mimalloc allocator.

Furthermore, in reftable/basics.c this #undef malloc is _specifically_ disabled by virtue of defining the REFTABLE_ALLOW_BANNED_ALLOCATORS constant before including reftable/basic.h, to ensure that such a custom allocator is also used in the reftable code.

However, in 8db127d43f5b (reftable: avoid leaks on realloc error, 2024-12-28) and in 2cca185e8517 (reftable: fix allocation count on realloc error, 2024-12-28), reftable_set_alloc() function calls were introduced that pass malloc, realloc and free function pointers as parameters _after_ reftable/basics.h ensured that they were no longer #defined. This would override the custom allocator and re-set it to the default allocator provided by, say, libc or MSVCRT.

This causes problems because those calls happen after the initial allocator has already been used to initialize an array, which is subsequently resized using the overridden default realloc() allocator.

You cannot mix and match allocators like that, which leads to a STATUS_HEAP_CORRUPTION (C0000374) on Windows, and when running this unit test through shell and/or prove (which only support 7-bit status codes), it surfaces as exit code 127.

It is actually unnecessary to use those function pointers to malloc/realloc/free, though: The reftable code goes out of its way to fall back to the initial allocator when passing NULL parameters instead. So let's do that instead of causing heap corruptions.

Signed-off-by Johannes Schindelin <[email protected]>

---
t-reftable-basics: allow for malloc to be #defined

This is a fix for one of the many issues that force me to delay Git for Windows v2.48.0-rc2 until I can increase my confidence via thorough testing.

The patch is based on rs/reftable-realloc-errors. Sadly, the patch fails the PR build
[https://github.com/gitgitgadget/git/actions/runs/12672507500/job/35316720255],
but then the base branch fails in the same way
[https://github.com/gitgitgadget/git/actions/runs/12533205564/job/34952668803].

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1848%2Fdscho%2Freftable-tests-should-allow-malloc-to-be-defined-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1848/dscho/reftable-tests-should-allow-malloc-to-be-defined-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1848

 t/unit-tests/t-reftable-basics.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

t/unit-tests/t-reftable-basics.c +2 -2

index 990dc1a2445..1d640b280f9 100644
@@ -157,13 +157,13 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 
 		old_alloc = alloc;
 		old_arr = arr;
-		reftable_set_alloc(malloc, realloc_stub, free);
+		reftable_set_alloc(NULL, realloc_stub, NULL);
 		check(REFTABLE_ALLOC_GROW(arr, old_alloc + 1, alloc));
 		check(arr == old_arr);
 		check_uint(alloc, ==, old_alloc);
 
 		old_alloc = alloc;
-		reftable_set_alloc(malloc, realloc, free);
+		reftable_set_alloc(NULL, NULL, NULL);
 		check(!REFTABLE_ALLOC_GROW(arr, old_alloc + 1, alloc));
 		check(arr != NULL);
 		check_uint(alloc, >, old_alloc);

t/unit-tests/t-reftable-basics.c +0 -0

 		reftable_free(arr);
 	}
 

base-commit: 1e781209284eb5952e153339f45bf0c1555e78bb

Quoted 4 lines

It is actually unnecessary to use those function pointers to malloc/realloc/free, though: The reftable code goes out of its way to fall back to the initial allocator when passing NULL parameters instead. So let's do that instead of causing heap corruptions.

Ugh. That makes a lot of sense. Sorry for the trouble! :-/