From: Phillip Wood <phillip.wood123@gmail.com>
Date: Wed, 06 Jul 2022 13:17:02 GMT
Subject: Re: [PATCH 2/3] xdiff: introduce XDL_CALLOC_ARRAY()
Message-ID: <114180f5-8c54-b6e1-14d1-4ad1a9666358@gmail.com>
In-Reply-To: <xmqqy1xednht.fsf@gitster.g>

Hi Junio

On 30/06/2022 19:17, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> diff --git a/xdiff/xmacros.h b/xdiff/xmacros.h
>> index 9fd3c5da91a..23db8e785d7 100644
>> --- a/xdiff/xmacros.h
>> +++ b/xdiff/xmacros.h
>> @@ -55,4 +55,10 @@ do { \
>>   		? xdl_malloc((nr) * sizeof(*(p)))	\
>>   		: NULL)
>>   
>> +/* Allocate an array of nr zeroed out elements, returns NULL on failure */
>> +#define XDL_CALLOC_ARRAY(p, nr)				\
>> +	(XDL_ALLOC_ARRAY((p), (nr))			\
>> +		? memset((p), 0, (nr) * sizeof(*(p)))	\
>> +		: NULL)
>> +
> 
> This implementation is somewhat dissapointing.  Allocating and then
> clearing is conceptually different from allocating an already
> cleared buffer.
> 
> Wouldn't it make more sense to build on top of xcalloc() or its
> counterpart in xdl world by introducing xdl_calloc()?  For that,
> this step would probably need to become two patches.  The first
> patch introduces xdl_calloc(), which uses xcalloc() in our codebase,
> and turn the existing "alloc and clear" code to use it, and the
> second patch would wrap the use of xdl_calloc() in the size-safe
> macro XDL_CALLOC_ARRAY().

I was hoping to avoid that by following the existing pattern of malloc() 
+ memset() but I'll reroll with a preparatory patch that converts the 
existing code to use xdl_calloc()

Best Wishes

Phillip