Re: [PATCH 5/8] mtrace: Wean away from malloc hooks

[Siddhesh Poyarekar <siddhesh@sourceware.org>]

On 6/25/21 4:43 AM, DJ Delorie via Libc-alpha wrote:
>> @@ -243,11 +262,14 @@ _calloc_debug_before (size_t *bytesp, void **victimp, const void *address)
>>   static __always_inline void *
>>   _calloc_debug_after (void *mem, size_t bytes, const void *address)
>>   {
>> -  if (__glibc_unlikely (__malloc_debugging_hooks) && mem != NULL)
>> +  if (__glibc_unlikely (__malloc_debugging_hooks))
>>       {
>>         if (__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK))
>>   	mem = malloc_mcheck_after (mem, bytes);
>> -      memset (mem, 0, bytes);
>> +      if (__is_malloc_debug_enabled (MALLOC_MTRACE_HOOK))
>> +	mem = malloc_mtrace_after (mem, bytes, address);
>> +      else if (mem != NULL)
>> +	memset (mem, 0, bytes);
>>       }
> 
> Why "else memset" ? We don't zero memory when tracing?  Or it's zero'd
> elsewhere?  What if we have both hooks set?
> 
> Regardless, deserves a comment here.

Ahh yes, that will break if mtrace hook is enabled along with another 
hook.  Mtrace itself does not need zeroing because it defers allocation 
and zeroing to glibc malloc.

I'll fix it up and also add a comment.

>> diff --git a/malloc/mtrace-hooks.c b/malloc/mtrace-hooks.c
>> new file mode 100644
>> index 0000000000..c1c6d6a6e9
>> --- /dev/null
>> +++ b/malloc/mtrace-hooks.c
> 
> Still missing makefile dependency.
> 
> Mostly just a copy-paste from mtrace.c, so OK.
> 

Fixed.

Thanks,
Siddhesh