From: Tobias Brunner <tobias@strongswan.org>
Date: Mon, 1 Sep 2014 08:22:39 +0000 (+0200)
Subject: array: RFC: Another approach to fix the remove_at() issue
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=625b311ed51a126ff3c69eb1acbf7010601d6d5e;p=thirdparty%2Fstrongswan.git

array: RFC: Another approach to fix the remove_at() issue

This is more efficient as it only requires a memcpy if remove_at is
actually called.  But is could be problematic if the passed pointer is
assigned to another variable or otherwise passed around before remove_at
is called.
---

diff --git a/src/libstrongswan/collections/array.c b/src/libstrongswan/collections/array.c
index dedbb25bf3..259e7059f7 100644
--- a/src/libstrongswan/collections/array.c
+++ b/src/libstrongswan/collections/array.c
@@ -211,8 +211,10 @@ typedef struct {
 	array_t *array;
 	/** current index +1, initialized at 0 */
 	int idx;
-	/** current element for element based arrays */
-	void *cur;
+	/** pointer passed to enumerate for element based arrays */
+	void **out;
+	/** removed element for element based arrays */
+	void *removed;
 } array_enumerator_t;
 
 METHOD(enumerator_t, enumerate, bool,
@@ -229,11 +231,11 @@ METHOD(enumerator_t, enumerate, bool,
 		  get_size(this->array, this->idx + this->array->head);
 	if (this->array->esize)
 	{
-		/* for element based arrays we copy the element and return a pointer to
-		 * the copy. this is required in case remove_at is called, which might
-		 * overwrite the current element */
-		memcpy(this->cur, pos, this->array->esize);
-		*out = this->cur;
+		/* for element based arrays we return a pointer to the element, but we
+		 * remember the passed pointer so we can redirect it when remove_at()
+		 * is called later */
+		*out = pos;
+		this->out = out;
 	}
 	else
 	{
@@ -247,7 +249,7 @@ METHOD(enumerator_t, enumerate, bool,
 METHOD(enumerator_t, enumerator_destroy, void,
 	array_enumerator_t *this)
 {
-	free(this->cur);
+	free(this->removed);
 	free(this);
 }
 
@@ -267,10 +269,6 @@ enumerator_t* array_create_enumerator(array_t *array)
 		},
 		.array = array,
 	);
-	if (array->esize)
-	{
-		enumerator->cur = malloc(array->esize);
-	}
 	return &enumerator->public;
 }
 
@@ -278,9 +276,25 @@ void array_remove_at(array_t *array, enumerator_t *public)
 {
 	array_enumerator_t *enumerator = (array_enumerator_t*)public;
 
-	if (enumerator->idx)
+	if (enumerator->idx--)
 	{
-		array_remove(array, --enumerator->idx, NULL);
+		if (array->esize)
+		{
+			void *pos;
+
+			/* for element based arrays we copy the element and change the
+			 * pointer passed to enumerate. this is required if array_remove(),
+			 * overwrites the current element */
+			if (!enumerator->removed)
+			{
+				enumerator->removed = malloc(array->esize);
+			}
+			*enumerator->out = enumerator->removed;
+			pos = enumerator->array->data + get_size(enumerator->array,
+									enumerator->idx + enumerator->array->head);
+			memcpy(enumerator->removed, pos, enumerator->array->esize);
+		}
+		array_remove(array, enumerator->idx, NULL);
 	}
 }