Re: [PATCH 2/3] xdiff: introduce XDL_CALLOC_ARRAY()

[Phillip Wood <phillip.wood123@gmail.com>]

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