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

[Junio C Hamano <gitster@pobox.com>]

"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?

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?

> +	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;
> +}