Convert all calls to rtrequest1() and the following error check

into a common pattern.  In the man page clarify the usage of the
returned route.
OK mpi@ mikeb@ jmc@

7 files changed, 63 insertions, 72 deletions

diff --git a/share/man/man9/rtrequest1.9 b/share/man/man9/rtrequest1.9
index 177ca8e68b2..fcbdc61137b 100644
--- a/share/man/man9/rtrequest1.9
+++ b/share/man/man9/rtrequest1.9
@@ -1,4 +1,4 @@
-.\"     $OpenBSD: rtrequest1.9,v 1.3 2014/10/15 11:58:13 mpi Exp $
+.\"     $OpenBSD: rtrequest1.9,v 1.4 2015/08/19 13:27:38 bluhm Exp $
 .\"
 .\" Copyright (c) 2011 Bret S. Lambert <blambert@openbsd.org>
 .\" All rights reserved.
@@ -15,7 +15,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: October 15 2014 $
+.Dd $Mdocdate: August 19 2015 $
 .Dt RTREQUEST1 9
 .Os
 .Sh NAME
@@ -80,12 +80,11 @@ and the requested action is
 then a default priority based on the priority of the associated
 interface is chosen.
 .It Fa rtp
-Points to the cloning entry if the action is
-.Dv RTM_RESOLVE
-or, if the action is
-.Dv RTM_DELETE
-and it is non-NULL, a pointer to the removed entry is placed there.
-In this case, the caller must take care of releasing the returned entry by
+Must be non-NULL and point to the cloning entry if the action is
+.Dv RTM_RESOLVE .
+In all cases when no error is returned and it is non-NULL, a pointer
+to the deleted or added entry is placed there.
+The caller must take care of releasing the returned reference by
 calling
 .Xr rtfree 9 .
 .It Fa rtableid
diff --git a/sys/net/route.c b/sys/net/route.c
index 8b613dd5bb0..c8d75fee77e 100644
--- a/sys/net/route.c
+++ b/sys/net/route.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.c,v 1.222 2015/08/19 10:42:37 mpi Exp $	*/
+/*	$OpenBSD: route.c,v 1.223 2015/08/19 13:27:38 bluhm Exp $	*/
 /*	$NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $	*/
 
 /*
@@ -301,9 +301,11 @@ struct rtentry *
 rtalloc(struct sockaddr *dst, int flags, unsigned int tableid)
 {
 	struct rtentry		*rt;
-	struct rtentry		*newrt = 0;
+	struct rtentry		*newrt = NULL;
 	struct rt_addrinfo	 info;
-	int			 s = splsoftnet(), err = 0, msgtype = RTM_MISS;
+	int			 s, error = 0, msgtype = RTM_MISS;
+
+	s = splsoftnet();
 
 	bzero(&info, sizeof(info));
 	info.rti_info[RTAX_DST] = dst;
@@ -312,14 +314,15 @@ rtalloc(struct sockaddr *dst, int flags, unsigned int tableid)
 	if (rt != NULL) {
 		newrt = rt;
 		if ((rt->rt_flags & RTF_CLONING) && ISSET(flags, RT_RESOLVE)) {
-			err = rtrequest1(RTM_RESOLVE, &info, RTP_DEFAULT,
+			error = rtrequest1(RTM_RESOLVE, &info, RTP_DEFAULT,
 			    &newrt, tableid);
-			if (err) {
+			if (error) {
 				newrt = rt;
 				rt->rt_refcnt++;
 				goto miss;
 			}
-			if ((rt = newrt) && (rt->rt_flags & RTF_XRESOLVE)) {
+			rt = newrt;
+			if (rt->rt_flags & RTF_XRESOLVE) {
 				msgtype = RTM_RESOLVE;
 				goto miss;
 			}
@@ -333,7 +336,7 @@ miss:
 		if (ISSET(flags, RT_REPORT)) {
 			bzero((caddr_t)&info, sizeof(info));
 			info.rti_info[RTAX_DST] = dst;
-			rt_missmsg(msgtype, &info, 0, NULL, err, tableid);
+			rt_missmsg(msgtype, &info, 0, NULL, error, tableid);
 		}
 	}
 	splx(s);
@@ -510,7 +513,7 @@ create:
 			rt = NULL;
 			error = rtrequest1(RTM_ADD, &info, RTP_DEFAULT, &rt,
 			    rdomain);
-			if (rt != NULL)
+			if (error == 0)
 				flags = rt->rt_flags;
 			stat = &rtstat.rts_dynamic;
 		} else {
@@ -1146,7 +1149,7 @@ int
 rt_ifa_add(struct ifaddr *ifa, int flags, struct sockaddr *dst)
 {
 	struct ifnet		*ifp = ifa->ifa_ifp;
-	struct rtentry		*rt = NULL;
+	struct rtentry		*rt;
 	struct sockaddr_rtlabel	 sa_rl;
 	struct rt_addrinfo	 info;
 	u_short			 rtableid = ifp->if_rdomain;
@@ -1179,7 +1182,7 @@ rt_ifa_add(struct ifaddr *ifa, int flags, struct sockaddr *dst)
 		prio = RTP_LOCAL;
 
 	error = rtrequest1(RTM_ADD, &info, prio, &rt, rtableid);
-	if (error == 0 && rt != NULL) {
+	if (error == 0) {
 		if (rt->rt_ifa != ifa) {
 			printf("%s: wrong ifa (%p) was (%p)\n", __func__,
 			    ifa, rt->rt_ifa);
@@ -1210,7 +1213,7 @@ int
 rt_ifa_del(struct ifaddr *ifa, int flags, struct sockaddr *dst)
 {
 	struct ifnet		*ifp = ifa->ifa_ifp;
-	struct rtentry		*rt, *nrt = NULL;
+	struct rtentry		*rt;
 	struct mbuf		*m = NULL;
 	struct sockaddr		*deldst;
 	struct rt_addrinfo	 info;
@@ -1261,11 +1264,11 @@ rt_ifa_del(struct ifaddr *ifa, int flags, struct sockaddr *dst)
 	if (flags & (RTF_LOCAL|RTF_BROADCAST))
 		prio = RTP_LOCAL;
 
-	error = rtrequest1(RTM_DELETE, &info, prio, &nrt, rtableid);
-	if (error == 0 && (rt = nrt) != NULL) {
-		rt_sendmsg(nrt, RTM_DELETE, rtableid);
+	error = rtrequest1(RTM_DELETE, &info, prio, &rt, rtableid);
+	if (error == 0) {
+		rt_sendmsg(rt, RTM_DELETE, rtableid);
 		if (flags & RTF_LOCAL)
-			rt_sendaddrmsg(nrt, RTM_DELADDR);
+			rt_sendaddrmsg(rt, RTM_DELADDR);
 		if (rt->rt_refcnt <= 0) {
 			rt->rt_refcnt++;
 			rtfree(rt);
diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c
index 538f2c08cb8..d95cbc6654a 100644
--- a/sys/net/rtsock.c
+++ b/sys/net/rtsock.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rtsock.c,v 1.167 2015/08/17 09:46:26 mpi Exp $	*/
+/*	$OpenBSD: rtsock.c,v 1.168 2015/08/19 13:27:38 bluhm Exp $	*/
 /*	$NetBSD: rtsock.c,v 1.18 1996/03/29 00:32:10 cgd Exp $	*/
 
 /*
@@ -595,7 +595,7 @@ route_output(struct mbuf *m, ...)
 		}
 		error = rtrequest1(rtm->rtm_type, &info, prio, &saved_nrt,
 		    tableid);
-		if (error == 0 && saved_nrt) {
+		if (error == 0) {
 			rt_setmetrics(rtm->rtm_inits, &rtm->rtm_rmx,
 			    &saved_nrt->rt_rmx);
 			/* write back the priority the kernel used */
diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
index c292213405d..8b671259b28 100644
--- a/sys/netinet/in_pcb.c
+++ b/sys/netinet/in_pcb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in_pcb.c,v 1.172 2015/07/19 02:35:35 deraadt Exp $	*/
+/*	$OpenBSD: in_pcb.c,v 1.173 2015/08/19 13:27:38 bluhm Exp $	*/
 /*	$NetBSD: in_pcb.c,v 1.25 1996/02/13 23:41:53 christos Exp $	*/
 
 /*
@@ -668,7 +668,7 @@ in_losing(struct inpcb *inp)
 		    inp->inp_rtableid);
 		if (rt->rt_flags & RTF_DYNAMIC)
 			(void)rtrequest1(RTM_DELETE, &info, rt->rt_priority,
-				(struct rtentry **)0, inp->inp_rtableid);
+			    NULL, inp->inp_rtableid);
 		/*
 		 * A new route can be allocated
 		 * the next time output is attempted.
diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
index 95657e3b8bb..866a8a74e91 100644
--- a/sys/netinet6/in6.c
+++ b/sys/netinet6/in6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in6.c,v 1.164 2015/08/19 11:09:24 mpi Exp $	*/
+/*	$OpenBSD: in6.c,v 1.165 2015/08/19 13:27:38 bluhm Exp $	*/
 /*	$KAME: in6.c,v 1.372 2004/06/14 08:14:21 itojun Exp $	*/
 
 /*
@@ -895,8 +895,8 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq *ifra,
 			info.rti_info[RTAX_NETMASK] = sin6tosa(&mltmask);
 			info.rti_info[RTAX_IFA] = sin6tosa(&ia6->ia_addr);
 			info.rti_flags = RTF_UP | RTF_CLONING;
-			error = rtrequest1(RTM_ADD, &info, RTP_CONNECTED,
-			    NULL, ifp->if_rdomain);
+			error = rtrequest1(RTM_ADD, &info, RTP_CONNECTED, NULL,
+			    ifp->if_rdomain);
 			if (error)
 				goto cleanup;
 		} else {
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index 47ebfc69ef4..31110312523 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6.c,v 1.144 2015/08/17 09:58:10 mpi Exp $	*/
+/*	$OpenBSD: nd6.c,v 1.145 2015/08/19 13:27:38 bluhm Exp $	*/
 /*	$KAME: nd6.c,v 1.280 2002/06/08 19:52:07 itojun Exp $	*/
 
 /*
@@ -646,7 +646,7 @@ nd6_lookup(struct in6_addr *addr6, int create, struct ifnet *ifp,
 	if (!rt) {
 		if (create && ifp) {
 			struct rt_addrinfo info;
-			int e;
+			int error;
 
 			/*
 			 * If no route is available and create is set,
@@ -671,18 +671,9 @@ nd6_lookup(struct in6_addr *addr6, int create, struct ifnet *ifp,
 			info.rti_info[RTAX_DST] = sin6tosa(&sin6);
 			info.rti_info[RTAX_GATEWAY] =
 			    (struct sockaddr *)ifp->if_sadl;
-			if ((e = rtrequest1(RTM_ADD, &info, RTP_CONNECTED,
-			    &rt, rtableid)) != 0) {
-#if 0
-				char ip[INET6_ADDRSTRLEN];
-				log(LOG_ERR, "%s: failed to add route for a "
-				    "neighbor(%s), errno=%d\n", __func__,
-				    inet_ntop(AF_INET6, addr6, ip, sizeof(ip)),
-				    e);
-#endif
-				return (NULL);
-			}
-			if (rt == NULL)
+			error = rtrequest1(RTM_ADD, &info, RTP_CONNECTED, &rt,
+			    rtableid);
+			if (error)
 				return (NULL);
 			if (rt->rt_llinfo) {
 				struct llinfo_nd6 *ln =
diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c
index 2dadc22945a..aa88ae17302 100644
--- a/sys/netinet6/nd6_rtr.c
+++ b/sys/netinet6/nd6_rtr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6_rtr.c,v 1.115 2015/08/18 08:52:25 mpi Exp $	*/
+/*	$OpenBSD: nd6_rtr.c,v 1.116 2015/08/19 13:27:38 bluhm Exp $	*/
 /*	$KAME: nd6_rtr.c,v 1.97 2001/02/07 11:09:13 itojun Exp $	*/
 
 /*
@@ -602,7 +602,7 @@ defrouter_addreq(struct nd_defrouter *new)
 {
 	struct rt_addrinfo info;
 	struct sockaddr_in6 def, mask, gate;
-	struct rtentry *rt = NULL;
+	struct rtentry *rt;
 	int s;
 	int error;
 
@@ -625,12 +625,11 @@ defrouter_addreq(struct nd_defrouter *new)
 	s = splsoftnet();
 	error = rtrequest1(RTM_ADD, &info, RTP_DEFAULT, &rt,
 	    new->ifp->if_rdomain);
-	if (rt) {
+	if (error == 0) {
 		rt_sendmsg(rt, RTM_ADD, new->ifp->if_rdomain);
 		rtfree(rt);
-	}
-	if (error == 0)
 		new->installed = 1;
+	}
 	splx(s);
 	return;
 }
@@ -705,7 +704,8 @@ defrouter_delreq(struct nd_defrouter *dr)
 {
 	struct rt_addrinfo info;
 	struct sockaddr_in6 def, mask, gw;
-	struct rtentry *oldrt = NULL;
+	struct rtentry *rt;
+	int error;
 
 #ifdef DIAGNOSTIC
 	if (!dr)
@@ -728,17 +728,17 @@ defrouter_delreq(struct nd_defrouter *dr)
 	info.rti_info[RTAX_GATEWAY] = sin6tosa(&gw);
 	info.rti_info[RTAX_NETMASK] = sin6tosa(&mask);
 
-	rtrequest1(RTM_DELETE, &info, RTP_DEFAULT, &oldrt,
+	error = rtrequest1(RTM_DELETE, &info, RTP_DEFAULT, &rt,
 	    dr->ifp->if_rdomain);
-	if (oldrt) {
-		rt_sendmsg(oldrt, RTM_DELETE, dr->ifp->if_rdomain);
-		if (oldrt->rt_refcnt <= 0) {
+	if (error == 0) {
+		rt_sendmsg(rt, RTM_DELETE, dr->ifp->if_rdomain);
+		if (rt->rt_refcnt <= 0) {
 			/*
 			 * XXX: borrowed from the RTM_DELETE case of
 			 * rtrequest1().
 			 */
-			oldrt->rt_refcnt++;
-			rtfree(oldrt);
+			rt->rt_refcnt++;
+			rtfree(rt);
 		}
 	}
 
@@ -1778,10 +1778,10 @@ nd6_prefix_onlink(struct nd_prefix *pr)
 	struct ifnet *ifp = pr->ndpr_ifp;
 	struct sockaddr_in6 mask6;
 	struct nd_prefix *opr;
-	u_long rtflags;
-	int error = 0;
-	struct rtentry *rt = NULL;
+	struct rtentry *rt;
 	char addr[INET6_ADDRSTRLEN];
+	u_long rtflags;
+	int error;
 
 	/* sanity check */
 	if ((pr->ndpr_stateflags & NDPRF_ONLINK) != 0)
@@ -1856,7 +1856,7 @@ nd6_prefix_onlink(struct nd_prefix *pr)
 	info.rti_info[RTAX_NETMASK] = sin6tosa(&mask6);
 
 	error = rtrequest1(RTM_ADD, &info, RTP_CONNECTED, &rt, ifp->if_rdomain);
-	if (error == 0 && rt != NULL) {
+	if (error == 0) {
 		pr->ndpr_stateflags |= NDPRF_ONLINK;
 		rt_sendmsg(rt, RTM_ADD, ifp->if_rdomain);
 		rtfree(rt);
@@ -1869,12 +1869,12 @@ int
 nd6_prefix_offlink(struct nd_prefix *pr)
 {
 	struct rt_addrinfo info;
-	int error = 0;
 	struct ifnet *ifp = pr->ndpr_ifp;
 	struct nd_prefix *opr;
 	struct sockaddr_in6 sa6, mask6;
-	struct rtentry *rt = NULL;
+	struct rtentry *rt;
 	char addr[INET6_ADDRSTRLEN];
+	int error;
 
 	/* sanity check */
 	if ((pr->ndpr_stateflags & NDPRF_ONLINK) == 0) {
@@ -1898,14 +1898,14 @@ nd6_prefix_offlink(struct nd_prefix *pr)
 	bzero(&info, sizeof(info));
 	info.rti_info[RTAX_DST] = sin6tosa(&sa6);
 	info.rti_info[RTAX_NETMASK] = sin6tosa(&mask6);
+
 	error = rtrequest1(RTM_DELETE, &info, RTP_CONNECTED, &rt,
 	    ifp->if_rdomain);
 	if (error == 0) {
 		pr->ndpr_stateflags &= ~NDPRF_ONLINK;
 
 		/* report the route deletion to the routing socket. */
-		if (rt != NULL)
-			rt_sendmsg(rt, RTM_DELETE, ifp->if_rdomain);
+		rt_sendmsg(rt, RTM_DELETE, ifp->if_rdomain);
 
 		/*
 		 * There might be the same prefix on another interface,
@@ -1946,6 +1946,12 @@ nd6_prefix_offlink(struct nd_prefix *pr)
 				}
 			}
 		}
+
+		if (rt->rt_refcnt <= 0) {
+			/* XXX: we should free the entry ourselves. */
+			rt->rt_refcnt++;
+			rtfree(rt);
+		}
 	} else {
 		/* XXX: can we still set the NDPRF_ONLINK flag? */
 		nd6log((LOG_ERR,
@@ -1955,14 +1961,6 @@ nd6_prefix_offlink(struct nd_prefix *pr)
 		    pr->ndpr_plen, ifp->if_xname, error));
 	}
 
-	if (rt != NULL) {
-		if (rt->rt_refcnt <= 0) {
-			/* XXX: we should free the entry ourselves. */
-			rt->rt_refcnt++;
-			rtfree(rt);
-		}
-	}
-
 	return (error);
 }