Re: [PATCH 3/3] xdiff: introduce XDL_ALLOC_GROW()

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

Hi Junio

On 30/06/2022 19:32, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> +/*
>> + * Ensure array p can accommodate at least nr elements, growing the
>> + * array and updating alloc (which is the number of allocated
>> + * elements) as necessary. Frees p and returns -1 on failure, returns
>> + * 0 on success
>> + */
>> +#define XDL_ALLOC_GROW(p, nr, alloc)	\
>> +	(-!((nr) <= (alloc) ||		\
>> +	    ((p) = xdl_alloc_grow_helper((p), (nr), &(alloc), sizeof(*(p))))))
>> + ...
>> +
>> +void* xdl_alloc_grow_helper(void *p, long nr, long *alloc, size_t size)
>> +{
>> +	void *tmp = NULL;
>> +	size_t n = ((LONG_MAX - 16) / 2 >= *alloc) ? 2 * *alloc + 16 : LONG_MAX;
> 
> Not counting in size_t but in long?

Yes, that's to match the existing code.

> I assume that 16 mimics the ALLOW_GROW(), but ALLOW_GROW() grows by
> 1.5, not by 2, so these two are not exactly compatible.
> 
> The computation of 'n' tries to avoid arithmetic in "long"
> overflowing, but does it ensure that we actually grow if we truncate
> at LONG_MAX?  After *alloc grew to LONG_MAX by calling this helper
> once, if we need to grow again and call this helper, 'n' will again
> get LONG_MAX and we end up not growing at all, no?

Right but the caller can only request up to LONG_MAX items in nr. If we 
were to grow beyond that alloc would be truncated when we returned it to 
the caller.

The code here is written to fit in with xdiff using long where it would 
be better using size_t (changing that would be a fairly major undertaking).

Best Wishes

Phillip

>> +	if (nr > n)
>> +		n = nr;
>> +	if (SIZE_MAX / size >= n)
>> +		tmp = xdl_realloc(p, n * size);
>> +	if (tmp) {
>> +		*alloc = n;
>> +	} else {
>> +		xdl_free(p);
>> +		*alloc = 0;
>> +	}
>> +	return tmp;
>> +}
> 
>