Re: [PATCH 2/3] xdiff: introduce XDL_CALLOC_ARRAY()
Hi Junio
On 30/06/2022 19:17, Junio C Hamano wrote:
Show 28 quoted lines
> "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