[PATCH] malloc: Remove redundant NULL check

[Cupertino Miranda]

Hi Wilco,

On 30-07-2025 10:57, Wilco Dijkstra wrote:
> Hi Cupertino,
> 
>> I wonder if in glibc there is an assert macro we could use to better
>> protect this code from programmer mistakes.
>> Can we add asserts in malloc code ?
> 
> assert() is used but it's broken in that those asserts are not removed in
> release builds. So we need to fix that before we add asserts in hot paths.
> 
>> When revisiting this code, all I can think of is, let me set an assert
>> to protect 'e' from ever being NULL.
>>
>> The only reason this does not seg faults is that we only call this
>> function if the bin has at least one element. Well, this check is never
>> verified within the function context, making it very fragile from a
>> software engineering point of view.
> 
> Well if you deference 'e' unconditionally immediately after assigning it
> then that is evidence that it cannot ever be NULL!	
Sure, but the function takes as argument a pointer to a pointer, so you 
could mutate the content of *ep.
 From the caller perspective this requirement would not be obvious.

I can imagine myself making the assumption this function could be used 
in empty list (bin) and externally ignore to check that the function 
does not support it. We either move those checks to the function and 
penalize performance or assert and make sure there are huge warnings 
through testing if anyone eventually makes that mistake. :-)

glibc malloc is packed with this implicit requirements and forces anyone 
to understand the full stack before being able to introduce any 2 lines 
of code. Nothing wrong with that, but it also means that it is much 
easier for us to miss those mistakes in the reviews.

Cheers,
Cupertino


> 
> Cheers,
> Wilco