[Swan-dev] more code that looks suspicious

[D. Hugh Redelmeier]

In find_host_connection2:
		for (; c != NULL; c = c->hp_next) {
		   	DBG(DBG_CONTROLMORE,
				DBG_log("found policy = %s (%s)",
	                                bitnamesof(sa_policy_bit_names,
      		                        	c->policy),
			         	c->name));
			if (NEVER_NEGOTIATE(c->policy))
     		                continue;

		        if ((c->policy & policy) == policy)
		   		break;

                        if ((policy & POLICY_XAUTH) !=
        	                (c->policy & POLICY_XAUTH))
		                continue;
		}

Look at that last if.  If the test is true, the loop continues.
Otherwise, the loop continues.  In other words: it cannot have any
effect.

What was intended here?

The second-last test could have been written as LIN(policy, c->policy)
It means: policy is a subset of c->policy.

If what is wanted is that XAUTH match exactly, the second-last test
could be changed to:
	LIN(policy, c->policy) &&
	((policy ^ c->policy) & POLICY_XAUTH) == LEMPTY
Generalizing, POLICY_XAUTH could be changed to an lset_t containing
all policies that must be the same.

A test that requires fewer branches:
	((policy & ~c->policy) | ((policy ^ c->policy) & POLICY_XAUTH)) == LEMPTY
Probably a bad choice since it is hard to understand, but likely more
efficient on modern processors