From: Alan T. DeKok <aland@freeradius.org>
Date: Sat, 26 Nov 2011 15:08:52 +0000 (+0100)
Subject: Rearrange packet list code
X-Git-Tag: release_3_0_0_beta0~472
X-Git-Url: http://www.project-moonshot.org/gitweb/?a=commitdiff_plain;h=38bf153ff0f1ffdf4f3c2698e5c373898c8e2fb0;p=freeradius.git

Rearrange packet list code

yank no longer returns a pointer.  No one was using it, so that
work was unnecessary.

Re-arrange the code in fr_packet_cmp() so that fewer comparisons
are necessary to disambiiguate packets.

Remove workaround for bug #35 in packet_entry_cmp().  It is
no longer necessary.

The result is a somewhat faster on in-memory performance tests.
But the callgrind output stil lshows large blocks of time
spent handling the packet lists.  Those could be optimized
some more.
---

diff --git a/src/include/packet.h b/src/include/packet.h
index 315a4ee..74e2a33 100644
--- a/src/include/packet.h
+++ b/src/include/packet.h
@@ -51,8 +51,8 @@ RADIUS_PACKET **fr_packet_list_find(fr_packet_list_t *pl,
 				      RADIUS_PACKET *request);
 RADIUS_PACKET **fr_packet_list_find_byreply(fr_packet_list_t *pl,
 					      RADIUS_PACKET *reply);
-RADIUS_PACKET **fr_packet_list_yank(fr_packet_list_t *pl,
-				      RADIUS_PACKET *request);
+void fr_packet_list_yank(fr_packet_list_t *pl,
+			 RADIUS_PACKET *request);
 int fr_packet_list_num_elements(fr_packet_list_t *pl);
 int fr_packet_list_id_alloc(fr_packet_list_t *pl, int proto,
 			    RADIUS_PACKET *request, void **pctx);
diff --git a/src/lib/packet.c b/src/lib/packet.c
index 1065173..ad68ce2 100644
--- a/src/lib/packet.c
+++ b/src/lib/packet.c
@@ -138,21 +138,22 @@ int fr_packet_cmp(const RADIUS_PACKET *a, const RADIUS_PACKET *b)
 {
 	int rcode;
 
-	if (a->sockfd < b->sockfd) return -1;
-	if (a->sockfd > b->sockfd) return +1;
+	rcode = a->id - b->id;
+	if (rcode != 0) return rcode;
 
-	if (a->id < b->id) return -1;
-	if (a->id > b->id) return +1;
+	rcode = (int) a->src_port - (int) b->src_port;
+	if (rcode != 0) return rcode;
 
-	if (a->src_port < b->src_port) return -1;
-	if (a->src_port > b->src_port) return +1;
+	rcode = (int) a->dst_port - (int) b->dst_port;
+	if (rcode != 0) return rcode;
 
-	if (a->dst_port < b->dst_port) return -1;
-	if (a->dst_port > b->dst_port) return +1;
+	rcode = a->sockfd - b->sockfd;
+	if (rcode != 0) return rcode;
 
-	rcode = fr_ipaddr_cmp(&a->dst_ipaddr, &b->dst_ipaddr);
+	rcode = fr_ipaddr_cmp(&a->src_ipaddr, &b->src_ipaddr);
 	if (rcode != 0) return rcode;
-	return fr_ipaddr_cmp(&a->src_ipaddr, &b->src_ipaddr);
+
+	return fr_ipaddr_cmp(&a->dst_ipaddr, &b->dst_ipaddr);
 }
 
 int fr_inaddr_any(fr_ipaddr_t *ipaddr)
@@ -519,8 +520,6 @@ static int packet_entry_cmp(const void *one, const void *two)
 	const RADIUS_PACKET * const *a = one;
 	const RADIUS_PACKET * const *b = two;
 
-	if (!a || !*a || !b || !*b) return -1; /* work-around for bug #35 */
-
 	return fr_packet_cmp(*a, *b);
 }
 
@@ -622,19 +621,16 @@ RADIUS_PACKET **fr_packet_list_find_byreply(fr_packet_list_t *pl,
 }
 
 
-RADIUS_PACKET **fr_packet_list_yank(fr_packet_list_t *pl,
-				      RADIUS_PACKET *request)
+void fr_packet_list_yank(fr_packet_list_t *pl, RADIUS_PACKET *request)
 {
-	RADIUS_PACKET **packet_p;
-
-	if (!pl || !request) return NULL;
+	rbnode_t *node;
 
-	packet_p = rbtree_finddata(pl->tree, &request);
-	if (!packet_p) return NULL;
+	if (!pl || !request) return;
 
-	if (!rbtree_deletebydata(pl->tree, packet_p)) return NULL;
+	node = rbtree_find(pl->tree, &request);
+	if (!node) return;
 
-	return packet_p;
+	rbtree_delete(pl->tree, node);
 }
 
 int fr_packet_list_num_elements(fr_packet_list_t *pl)