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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio

On 30/06/2022 19:32, Junio C Hamano wrote:
"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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;
+}





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux