Subject: Re: Last remaining insque / remque uses
To: David Young <dyoung@pobox.com>
From: Iain Hibbert <plunky@rya-online.net>
List: tech-kern
Date: 10/31/2007 06:24:14
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, 26 Oct 2007, David Young wrote:

> On Fri, Oct 26, 2007 at 06:33:17PM +0100, Iain Hibbert wrote:
> > Having looked at it in a bit more depth, I think that if no testers can be
> > found for ISO/EON/TPIP then just hardcoding it is the cheapest option and
> > is least likley to break anything not already broken, eg:
>
> Please, don't do that.  Use queue(3) macros.  They work, developers
> believe they work, and LIST_INSERT_HEAD() will reduce the lines of code
> and comments in that example from five to one.

Ok yeah, it was ugly I agree.. (but cheap! :)

In the case of netiso, using queue(3) macros increases the amount of
work by an order of magnitude and the risk of breakage becomes much higher
(with no one to say if its gone wrong).  I'm hacking on it but I
think that queueing mechanism lent itself to abuse and the opportunity
seems to have been taken a couple of times, so its slow going :)

However, if the usage is removed from coda as per my other suggestion,
then this is the only thing left using insque/remque - we could do as Tom
suggested and embed them inside the netiso code as below. Its somewhat
cleaner and just as cheap - but is it good enough?

iain

- --- /usr/src/sys/netiso/if_eon.c	2007-10-24 20:21:10.000000000 +0100
+++ if_eon.c	2007-10-28 19:02:07.000000000 +0000
@@ -266,7 +266,7 @@
 	switch (cmd) {
 	case RTM_DELETE:
 		if (el) {
- -			remque(&el->el_qhdr);
+			iso_remque(&el->el_qhdr);
 			rtcache_free(&el->el_iproute);
 			Free(el);
 			rt->rt_llinfo = NULL;
@@ -281,7 +281,7 @@
 		if (el == NULL)
 			return;
 		memset(el, 0, sizeof(*el));
- -		insque(&el->el_qhdr, &eon_llinfo.el_qhdr);
+		iso_insque(&el->el_qhdr, &eon_llinfo.el_qhdr);
 		el->el_rt = rt;
 		break;
 	}
- --- /usr/src/sys/netiso/iso.c	2007-07-20 22:24:52.000000000 +0100
+++ iso.c	2007-10-28 19:00:03.000000000 +0000
@@ -952,3 +952,47 @@
 }

 #endif /* ARGO_DEBUG */
+
+struct queue {
+	struct queue *q_next, *q_prev;
+};
+
+/*
+ * FUNCTION:		iso_insque
+ *
+ * PURPOSE:		insert an element into a queue
+ *
+ * RETURNS:
+ */
+void
+iso_insque(void *v1, void *v2)
+{
+	struct queue *elem = v1, *head = v2;
+	struct queue *next;
+
+	next = head->q_next;
+	elem->q_next = next;
+	head->q_next = elem;
+	elem->q_prev = head;
+	next->q_prev = elem;
+}
+
+/*
+ * FUNCTION:		iso_remque
+ *
+ * PURPOSE:		remove an element from a queue
+ *
+ * RETURNS:
+ */
+void
+iso_remque(void *v)
+{
+	struct queue *elem = v;
+	struct queue *next, *prev;
+
+	next = elem->q_next;
+	prev = elem->q_prev;
+	next->q_prev = prev;
+	prev->q_next = next;
+	elem->q_prev = NULL;
+}
- --- /usr/src/sys/netiso/iso_pcb.c	2007-07-20 22:24:52.000000000 +0100
+++ iso_pcb.c	2007-10-28 19:02:11.000000000 +0000
@@ -118,7 +118,7 @@
 		return ENOBUFS;
 	isop->isop_head = head;
 	isop->isop_socket = so;
- -	insque(isop, head);
+	iso_insque(isop, head);
 	if (so)
 		so->so_pcb = isop;
 	return 0;
@@ -545,7 +545,7 @@
 		printf("iso_pcbdetach 4 \n");
 	}
 #endif
- -	remque(isop);
+	iso_remque(isop);
 #ifdef ARGO_DEBUG
 	if (argo_debug[D_ISO]) {
 		printf("iso_pcbdetach 5 \n");
- --- /usr/src/sys/netiso/iso_var.h	2007-07-20 22:24:52.000000000 +0100
+++ iso_var.h	2007-10-28 19:20:12.000000000 +0000
@@ -160,6 +160,8 @@
 struct iso_ifaddr *iso_localifa (const struct sockaddr_iso *);
 int iso_nlctloutput (int, int, void *, struct mbuf *);
 void dump_isoaddr(const struct sockaddr_iso *);
+void iso_insque(void *, void *);
+void iso_remque(void *);

 /* iso_chksum.c */
 int iso_check_csum (struct mbuf *, int);
- --- /usr/src/sys/netiso/tp_input.c	2007-03-31 14:39:35.000000000 +0100
+++ tp_input.c	2007-10-28 19:02:15.000000000 +0000
@@ -899,7 +899,7 @@
 				goto respond;
 			}
 			tpcb = sototpcb(so);
- -			insque(tpcb, parent_tpcb);
+			iso_insque(tpcb, parent_tpcb);

 			/*
 			 * Stash the addresses in the net level pcb
- --- /usr/src/sys/netiso/tp_output.c	2007-07-20 22:24:52.000000000 +0100
+++ tp_output.c	2007-10-28 19:01:17.000000000 +0000
@@ -534,7 +534,7 @@
 			tpcb->tp_lsuffixlen = 0;
 			tpcb->tp_state = TP_LISTENING;
 			error = 0;
- -			remque(tpcb);
+			iso_remque(tpcb);
 			tpcb->tp_next = tpcb->tp_prev = tpcb;
 			tpcb->tp_nextlisten = tp_listeners;
 			tp_listeners = tpcb;
- --- /usr/src/sys/netiso/tp_pcb.c	2007-03-31 14:39:35.000000000 +0100
+++ tp_pcb.c	2007-10-28 19:02:18.000000000 +0000
@@ -809,7 +809,7 @@
 		tp_rsyflush(tpcb);

 	if (tpcb->tp_next) {
- -		remque(tpcb);
+		iso_remque(tpcb);
 		tpcb->tp_next = tpcb->tp_prev = 0;
 	}
 	tpcb->tp_notdetached = 0;
@@ -996,7 +996,7 @@
 				}
 		}
 		bcopy(tsel, tpcb->tp_lsuffix, (tpcb->tp_lsuffixlen = tlen));
- -		insque(tpcb, &tp_bound_pcbs);
+		iso_insque(tpcb, &tp_bound_pcbs);
 	} else {
 		if (tlen || siso == 0)
 			return (EINVAL);
- --- /usr/src/sys/netiso/tp_subr2.c	2007-03-31 14:39:35.000000000 +0100
+++ tp_subr2.c	2007-10-28 19:01:24.000000000 +0000
@@ -656,7 +656,7 @@
 		 * level options or done a pcbconnect and XXXXXXX'edly apply
 		 * to both inpcb's and isopcb's
 		 */
- -		remque(isop_new);
+		iso_remque(isop_new);
 		free(isop_new, M_PCB);
 		tpcb->tp_npcb = (void *) isop;
 		tpcb->tp_netservice = ISO_CONS;
- --- /usr/src/sys/netiso/tp_usrreq.c	2007-03-31 14:39:35.000000000 +0100
+++ tp_usrreq.c	2007-10-28 19:34:14.000000000 +0000
@@ -90,6 +90,7 @@
 #include <netiso/tp_meas.h>
 #include <netiso/iso.h>
 #include <netiso/iso_errno.h>
+#include <netiso/iso_var.h>

 int             TNew;
 int             TPNagle1, TPNagle2;
@@ -455,7 +456,7 @@
 			error = EINVAL;
 		else {
 			struct tp_pcb **tt;
- -			remque(tpcb);
+			iso_remque(tpcb);
 			tpcb->tp_next = tpcb->tp_prev = tpcb;
 			for (tt = &tp_listeners; *tt; tt = &((*tt)->tp_nextlisten))
 				if ((*tt)->tp_lsuffixlen)


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (NetBSD)

iQEVAwUBRygfj/FJxoMWDXVDAQJO8QgAu3UL+nS6/wnnz8xReezjYGR6rYIEJ2x7
BsNOv19V7IigeRZkvkAiJogmd/2tQTSFcAAxQZ0IajyPW+BdewojJyMJ52df+gzE
IZLDR2eE3YZ0s3YlhKAt+YN/g4qRqjfUNfJ1CCAEGQG7yDkFRhqqCK0li7/pVcOd
/Ihm2gyfx2syYXhR1EepHlSgHY7QbU83hzJG5S8OOmHh30bBHvQcc0qs5kaoRVwZ
+/Bp9wd2fliuRM4TvYnoq0yrfDtOyfcdwoj90jaS2Zc/YQqV1T7g8TOuUubZ1YuQ
PgoeOlmlSuZvSQ9p5O0D0avdHWw7g3axre3ls0arzl0bmAixccgZfw==
=y3b3
-----END PGP SIGNATURE-----