Re: [PATCH] mem-pool: fix big allocations

From: René Scharfe <l.s.r@web.de>
Date: Dec 28, 2023, 16:05 UTC
Message-ID: <c5d35735-10e2-4b71-8fc7-6218e7002549@web.de>
In-Reply-To: <9aad15c8-8d3b-475b-bd44-5d24121cb793@gmail.com>

Am 28.12.23 um 16:10 schrieb Phillip Wood:

Show 22 quoted lines

> Hi René
>
> On 21/12/2023 23:13, René Scharfe wrote:
>> +#define check_ptr(a, op, b) check_int(((a) op (b)), ==, 1)
>> [...]
>> +static void t_calloc_100(struct mem_pool *pool)
>> +{
>> +    size_t size = 100;
>> +    char *buffer = mem_pool_calloc(pool, 1, size);
>> +    for (size_t i = 0; i < size; i++)
>> +        check_int(buffer[i], ==, 0);
>> +    if (!check_ptr(pool->mp_block, !=, NULL))
>> +        return;
>> +    check_ptr(pool->mp_block->next_free, <=, pool->mp_block->end);
>> +    check_ptr(pool->mp_block->next_free, !=, NULL);
>> +    check_ptr(pool->mp_block->end, !=, NULL);
>> +}
>
> It's great to see the unit test framework being used here. I wonder
> though if it would be simpler just to use
>
>     check(ptr != NULL)

Yes, that's better.

Show 10 quoted lines

> as I'm not sure what the check_ptr() macro adds. The diff at the end of
> this email shows a possible implementation of a check_ptr() macro for
> the unit test library. I'm wary of adding it though because I'm not sure
> printing the pointer values is actually very useful most of the
> time. I'm also concerned that the rules around pointer arithmetic and
> comparisons mean that many pointer tests such as
>
>     check_ptr(pool->mp_block->next_free, <=, pool->mp_block->end);
>
> will be undefined if they fail.

True, the compiler could legally emit mush when it finds out that the pointers are for different objects. And the error being fixed produces such unrelated pointer pairs -- oops.

This check is not important here, we can just drop it.

mem_pool_contains() has the same problem, by the way.

Restricting ourselves to only equality comparisons for pointers prevents some interesting sanity checks, though. Casting to intptr_t or uintptr_t would allow arbitrary comparisons without risk of undefined behavior, though. Perhaps that would make a check_ptr() macro viable and useful.

René