From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: use FOREACH_SAFE instead of handrolling it
To: tech@openbsd.org
Date: Wed, 12 Feb 2025 17:12:41 +0100

In most cases we use the FOREACH_SAFE iterator with the various queues
when needed but some old code still used the explicit for loop.
This changes those.

The changes in name2id.c are less obvious. In _unref() using FOREACH_SAFE
is actually not really needed (since we break out before using the next
element). In _name2id the construct is impossible to read and I think the
new form is hopefully easier to read.

Two loops are left, one in rde.c and one in rde_filter.c which are so
special that I don't want to change them.

-- 
:wq Claudio

Index: config.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
diff -u -p -r1.113 config.c
--- config.c	13 Dec 2024 19:21:03 -0000	1.113
+++ config.c	12 Feb 2025 15:55:49 -0000
@@ -407,9 +407,7 @@ merge_config(struct bgpd_config *xconf, 
 			if (ola->flags & DEFAULT_LISTENER)
 				ola->reconf = RECONF_KEEP;
 	/* else loop over listeners and merge configs */
-	for (nla = TAILQ_FIRST(conf->listen_addrs); nla != NULL; nla = next) {
-		next = TAILQ_NEXT(nla, entry);
-
+	TAILQ_FOREACH_SAFE(nla, conf->listen_addrs, entry, next) {
 		TAILQ_FOREACH(ola, xconf->listen_addrs, entry)
 			if (!memcmp(&nla->sa, &ola->sa, sizeof(nla->sa)))
 				break;
@@ -423,8 +421,7 @@ merge_config(struct bgpd_config *xconf, 
 			ola->reconf = RECONF_KEEP;
 	}
 	/* finally clean up the original list and remove all stale entries */
-	for (nla = TAILQ_FIRST(xconf->listen_addrs); nla != NULL; nla = next) {
-		next = TAILQ_NEXT(nla, entry);
+	TAILQ_FOREACH_SAFE(nla, xconf->listen_addrs, entry, next) {
 		if (nla->reconf == RECONF_DELETE) {
 			TAILQ_REMOVE(xconf->listen_addrs, nla, entry);
 			free(nla);
@@ -568,8 +565,7 @@ prepare_listeners(struct bgpd_config *co
 	int			 opt = 1;
 	int			 r = 0;
 
-	for (la = TAILQ_FIRST(conf->listen_addrs); la != NULL; la = next) {
-		next = TAILQ_NEXT(la, entry);
+	TAILQ_FOREACH_SAFE(la, conf->listen_addrs, entry, next) {
 		if (la->reconf != RECONF_REINIT)
 			continue;
 
Index: mrt.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
diff -u -p -r1.124 mrt.c
--- mrt.c	31 Jan 2025 20:07:18 -0000	1.124
+++ mrt.c	12 Feb 2025 15:56:27 -0000
@@ -1185,8 +1185,7 @@ mrt_reconfigure(struct mrt_head *mrt)
 	time_t		 now;
 
 	now = time(NULL);
-	for (m = LIST_FIRST(mrt); m != NULL; m = xm) {
-		xm = LIST_NEXT(m, entry);
+	LIST_FOREACH_SAFE(m, mrt, entry, xm) {
 		if (m->state == MRT_STATE_OPEN ||
 		    m->state == MRT_STATE_REOPEN) {
 			if (mrt_open(m, now) == -1)
Index: name2id.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/name2id.c,v
diff -u -p -r1.12 name2id.c
--- name2id.c	16 Jun 2022 15:30:12 -0000	1.12
+++ name2id.c	12 Feb 2025 16:04:08 -0000
@@ -119,10 +119,11 @@ _name2id(struct n2id_labels *head, const
 	 * is empty, append a new entry at the end.
 	 */
 
-	if (!TAILQ_EMPTY(head))
-		for (p = TAILQ_FIRST(head); p != NULL &&
-		    p->id == new_id; p = TAILQ_NEXT(p, entry))
-			new_id = p->id + 1;
+	TAILQ_FOREACH(p, head, entry) {
+		if (p->id != new_id)
+			break;
+		new_id++;
+	}
 
 	if (new_id > IDVAL_MAX)
 		return (0);
@@ -167,8 +168,7 @@ _unref(struct n2id_labels *head, uint16_
 	if (id == 0)
 		return;
 
-	for (p = TAILQ_FIRST(head); p != NULL; p = next) {
-		next = TAILQ_NEXT(p, entry);
+	TAILQ_FOREACH_SAFE(p, head, entry, next) {
 		if (id == p->id) {
 			if (--p->ref == 0) {
 				TAILQ_REMOVE(head, p, entry);
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.651 rde.c
--- rde.c	4 Feb 2025 18:16:56 -0000	1.651
+++ rde.c	12 Feb 2025 15:53:02 -0000
@@ -243,9 +243,8 @@ rde_main(int debug, int verbose)
 		set_pollfd(&pfd[PFD_PIPE_ROA], ibuf_rtr);
 
 		i = PFD_PIPE_COUNT;
-		for (mctx = LIST_FIRST(&rde_mrts); mctx != 0; mctx = xmctx) {
-			xmctx = LIST_NEXT(mctx, entry);
 
+		LIST_FOREACH_SAFE(mctx, &rde_mrts, entry, xmctx) {
 			if (i >= pfd_elms)
 				fatalx("poll pfd too small");
 			if (msgbuf_queuelen(mctx->mrt.wbuf) > 0) {
@@ -301,7 +300,7 @@ rde_main(int debug, int verbose)
 			rde_dispatch_imsg_rtr(ibuf_rtr);
 
 		for (j = PFD_PIPE_COUNT, mctx = LIST_FIRST(&rde_mrts);
-		    j < i && mctx != 0; j++) {
+		    j < i && mctx != NULL; j++) {
 			if (pfd[j].fd == mctx->mrt.fd &&
 			    pfd[j].revents & POLLOUT)
 				mrt_write(&mctx->mrt);
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.514 session.c
--- session.c	12 Feb 2025 13:10:13 -0000	1.514
+++ session.c	12 Feb 2025 15:53:02 -0000
@@ -295,8 +295,7 @@ session_main(int debug, int verbose)
 		}
 
 		mrt_cnt = 0;
-		for (m = LIST_FIRST(&mrthead); m != NULL; m = xm) {
-			xm = LIST_NEXT(m, entry);
+		LIST_FOREACH_SAFE(m, &mrthead, entry, xm) {
 			if (m->state == MRT_STATE_REMOVE) {
 				mrt_clean(m);
 				LIST_REMOVE(m, entry);