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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 30, 2022, 18:32 UTC
Message-ID: <xmqqtu82dmu2.fsf@gitster.g>
In-Reply-To: <da996677f0730ec7a50d399524fb58c44dad468a.1656516334.git.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 15 quoted lines

> +/*
> + * 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?

Show 12 quoted lines

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