Merge branch 'variadic-enumerators'

This adds several changes to enumerator_t and linked_list_t to improve
portability.  In particular to Apple's ARM64 iOS platform, whose calling
convention for variadic and regular functions are different.  This means
that assigning a non-variadic function to a variadic function pointer,
as we did with our enumerator_t::enumerate() implementations and several
callbacks, will result in crashes as the called function will access the
arguments differently than the caller provided them.

To avoid this issue the enumerator_t interface is now fully variadic.
A new mandatory method is added, venumerate(), that takes a va_list with
the arguments provided while enumerating.  enumerate() is replaced with
a generic implementation that prepares a va_list and calls the
enumerator's venumerate() implementation.  As this allows passing the
arguments of one enumerator to another it avoids the five pointer hack
used by enumerator_create_nested() and enumerator_create_cleaner().
To simplify the implementation of venumerate() a helper macro is provided
that assigns values from a given va_list to local variables.

The signature of the callback passed to enumerator_create_filter() has
also changed significantly.  It's now required to enumerate over the
original enumerator in the callback as this avoids the previous in/out
pointer hack. The arguments to the outer enumerator are provided in a
va_list.

Similar changes to avoid such five pointer hacks affect the signatures
of the callbacks for linked_list_t's invoke_function() and find_first()
methods.  For the latter the return type also changed from status_t to
bool, which is important as SUCCESS is defined as 0, so checks for ==
SUCCESS will now fail.

1 files changed, 46 insertions, 34 deletions

diff --git a/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c
index fd427ebbb..fd1adb2ae 100644
--- a/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c
+++ b/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c
@@ -464,10 +464,10 @@ static policy_sa_t *policy_sa_create(private_kernel_pfkey_ipsec_t *this,
 /**
  * Destroy a policy_sa(_in)_t object
  */
-static void policy_sa_destroy(policy_sa_t *policy, policy_dir_t *dir,
+static void policy_sa_destroy(policy_sa_t *policy, policy_dir_t dir,
 							  private_kernel_pfkey_ipsec_t *this)
 {
-	if (*dir == POLICY_OUT)
+	if (dir == POLICY_OUT)
 	{
 		policy_sa_out_t *out = (policy_sa_out_t*)policy;
 		out->src_ts->destroy(out->src_ts);
@@ -477,6 +477,16 @@ static void policy_sa_destroy(policy_sa_t *policy, policy_dir_t *dir,
 	free(policy);
 }
 
+CALLBACK(policy_sa_destroy_cb, void,
+	policy_sa_t *policy, va_list args)
+{
+	private_kernel_pfkey_ipsec_t *this;
+	policy_dir_t dir;
+
+	VA_ARGS_VGET(args, dir, this);
+	policy_sa_destroy(policy, dir, this);
+}
+
 typedef struct policy_entry_t policy_entry_t;
 
 /**
@@ -557,9 +567,8 @@ static void policy_entry_destroy(policy_entry_t *policy,
 	}
 	if (policy->used_by)
 	{
-		policy->used_by->invoke_function(policy->used_by,
-										(linked_list_invoke_t)policy_sa_destroy,
-										 &policy->direction, this);
+		policy->used_by->invoke_function(policy->used_by, policy_sa_destroy_cb,
+										 policy->direction, this);
 		policy->used_by->destroy(policy->used_by);
 	}
 	DESTROY_IF(policy->src.net);
@@ -567,12 +576,21 @@ static void policy_entry_destroy(policy_entry_t *policy,
 	free(policy);
 }
 
-/**
- * compares two policy_entry_t
- */
-static inline bool policy_entry_equals(policy_entry_t *current,
-									   policy_entry_t *policy)
+CALLBACK(policy_entry_destroy_cb, void,
+	policy_entry_t *policy, va_list args)
 {
+	private_kernel_pfkey_ipsec_t *this;
+
+	VA_ARGS_VGET(args, this);
+	policy_entry_destroy(policy, this);
+}
+
+CALLBACK(policy_entry_equals, bool,
+	policy_entry_t *current, va_list args)
+{
+	policy_entry_t *policy;
+
+	VA_ARGS_VGET(args, policy);
 	return current->direction == policy->direction &&
 		   current->src.proto == policy->src.proto &&
 		   current->dst.proto == policy->dst.proto &&
@@ -582,13 +600,13 @@ static inline bool policy_entry_equals(policy_entry_t *current,
 		   current->dst.net->equals(current->dst.net, policy->dst.net);
 }
 
-/**
- * compare the given kernel index with that of a policy
- */
-static inline bool policy_entry_match_byindex(policy_entry_t *current,
-											  uint32_t *index)
+CALLBACK(policy_entry_match_byindex, bool,
+	policy_entry_t *current, va_list args)
 {
-	return current->index == *index;
+	uint32_t index;
+
+	VA_ARGS_VGET(args, index);
+	return current->index == index;
 }
 
 /**
@@ -1261,9 +1279,8 @@ static void process_acquire(private_kernel_pfkey_ipsec_t *this,
 
 	index = response.x_policy->sadb_x_policy_id;
 	this->mutex->lock(this->mutex);
-	if (this->policies->find_first(this->policies,
-								(linked_list_match_t)policy_entry_match_byindex,
-								(void**)&policy, &index) == SUCCESS &&
+	if (this->policies->find_first(this->policies, policy_entry_match_byindex,
+								  (void**)&policy, index) &&
 		policy->used_by->get_first(policy->used_by, (void**)&sa) == SUCCESS)
 	{
 		reqid = sa->sa->cfg.reqid;
@@ -2554,8 +2571,7 @@ static status_t add_policy_internal(private_kernel_pfkey_ipsec_t *this,
 
 	/* we try to find the policy again and update the kernel index */
 	this->mutex->lock(this->mutex);
-	if (this->policies->find_first(this->policies, NULL,
-								  (void**)&policy) != SUCCESS)
+	if (!this->policies->find_first(this->policies, NULL, (void**)&policy))
 	{
 		DBG2(DBG_KNL, "unable to update index, the policy is already gone, "
 					  "ignoring");
@@ -2606,9 +2622,8 @@ METHOD(kernel_ipsec_t, add_policy, status_t,
 
 	/* find a matching policy */
 	this->mutex->lock(this->mutex);
-	if (this->policies->find_first(this->policies,
-								  (linked_list_match_t)policy_entry_equals,
-								  (void**)&found, policy) == SUCCESS)
+	if (this->policies->find_first(this->policies, policy_entry_equals,
+								   (void**)&found, policy))
 	{	/* use existing policy */
 		DBG2(DBG_KNL, "policy %R === %R %N already exists, increasing "
 			 "refcount", id->src_ts, id->dst_ts, policy_dir_names, id->dir);
@@ -2701,9 +2716,8 @@ METHOD(kernel_ipsec_t, query_policy, status_t,
 
 	/* find a matching policy */
 	this->mutex->lock(this->mutex);
-	if (this->policies->find_first(this->policies,
-								  (linked_list_match_t)policy_entry_equals,
-								  (void**)&found, policy) != SUCCESS)
+	if (!this->policies->find_first(this->policies, policy_entry_equals,
+									(void**)&found, policy))
 	{
 		DBG1(DBG_KNL, "querying policy %R === %R %N failed, not found",
 			 id->src_ts, id->dst_ts, policy_dir_names, id->dir);
@@ -2814,9 +2828,8 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
 
 	/* find a matching policy */
 	this->mutex->lock(this->mutex);
-	if (this->policies->find_first(this->policies,
-								  (linked_list_match_t)policy_entry_equals,
-								  (void**)&found, policy) != SUCCESS)
+	if (!this->policies->find_first(this->policies, policy_entry_equals,
+									(void**)&found, policy))
 	{
 		DBG1(DBG_KNL, "deleting policy %R === %R %N failed, not found",
 			 id->src_ts, id->dst_ts, policy_dir_names, id->dir);
@@ -2860,7 +2873,7 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
 	if (policy->used_by->get_count(policy->used_by) > 0)
 	{	/* policy is used by more SAs, keep in kernel */
 		DBG2(DBG_KNL, "policy still used by another CHILD_SA, not removed");
-		policy_sa_destroy(mapping, &id->dir, this);
+		policy_sa_destroy(mapping, id->dir, this);
 
 		if (!is_installed)
 		{	/* no need to update as the policy was not installed for this SA */
@@ -2915,7 +2928,7 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
 	}
 
 	this->policies->remove(this->policies, found, NULL);
-	policy_sa_destroy(mapping, &id->dir, this);
+	policy_sa_destroy(mapping, id->dir, this);
 	policy_entry_destroy(policy, this);
 	this->mutex->unlock(this->mutex);
 
@@ -3088,8 +3101,7 @@ METHOD(kernel_ipsec_t, destroy, void,
 		lib->watcher->remove(lib->watcher, this->socket_events);
 		close(this->socket_events);
 	}
-	this->policies->invoke_function(this->policies,
-								   (linked_list_invoke_t)policy_entry_destroy,
+	this->policies->invoke_function(this->policies, policy_entry_destroy_cb,
 									this);
 	this->policies->destroy(this->policies);
 	this->excludes->destroy(this->excludes);