[Openswan dev] find_host_connection2

D. Hugh Redelmeier hugh at mimosa.com
Sun Jul 5 22:08:08 EDT 2009


I was trying to understand the code for find_host_connection2 in 
programs/pluto/connections.c.  I'm looking in the version in openswan 
2.6.22.

It looks to me that the code could be simplified.  Look at the
enclosed diff.

NOTE 1: this is untested.  I could be quite wrong.

NOTE 2: this does not include the fix by hiren joshi that I agreed
with in my previous post.

NOTE 3: To reduce the diff, I didn't change the indentation where it should
be.

I don't think that POLICY == LEMPTY needs to be treated specially.  I
think that the code just works either way.  Why?  Because
	(any_policy & LEMPTY) != LEMPTY
is always false, so the loop will break at the first !NEVER_NEGOTIATE.

This also eliminates the need for a special second for loop to skip
NEVER_NEGOTIATE entries.

The word "found" should have been "considering".  With this change, it
actually becomes "rejecting".

I don't like the look of control statements like for on if having
their subject command on the same line.  I eliminated those without
just reformatting (and therefore avoiding a religeous argument).

--- programs/pluto/connections.c	2009-06-22 22:53:08.000000000 -0400
+++ programs/pluto/connections.c.simpler	2009-07-05 21:51:16.000000000 -0400
@@ -2203,27 +2203,19 @@
 		, his_port , bitnamesof(sa_policy_bit_names, policy)));
     c = find_host_pair_connections(__FUNCTION__, me, my_port, him, his_port);
 
-    if (policy != LEMPTY) {
 	/*
 	 * if we have requirements for the policy, choose the first matching
 	 * connection.
 	 */
-	for (; c != NULL; c = c->hp_next) {
+	while (c != NULL && (NEVER_NEGOTIATE(c->policy) || (c->policy & policy) != policy)) {
 	    DBG(DBG_CONTROLMORE,
-		DBG_log("searching for policy=%s, found=%s (%s)" 
+		DBG_log("searching for policy=%s, rejecting=%s (%s)" 
 			, bitnamesof(sa_policy_bit_names, policy)
 			, bitnamesof(sa_policy_bit_names, c->policy)
 			, c->name));
-	    if(NEVER_NEGOTIATE(c->policy)) continue;
-
-	    if ((c->policy & policy) == policy)
-		break;
+	    c = c->hp_next;
 	}
 
-    }
-
-    for(; c != NULL && NEVER_NEGOTIATE(c->policy); c = c->hp_next);
-
     DBG(DBG_CONTROLMORE,
 	DBG_log("find_host_connection returns %s", c ? c->name : "empty"));
     return c;
================ end ================


More information about the Dev mailing list