From 0039ae5121f443fb1a694aa0cd205482e690200f Mon Sep 17 00:00:00 2001 From: jca Date: Fri, 18 Apr 2014 10:48:29 +0000 Subject: [PATCH] Invert the signature logic of in{,6}_selectsrc, make them return the error code and pass the resulting source address back to the caller through a pointer, as suggested by chrisz. This gives us more readable code, and eases the deletion of useless checks in the callers' error path. Add a bunch of "0 -> NULL" conversions, while here. ok chrisz@ mpi@ --- sys/netinet/in_pcb.c | 44 ++++++++-------- sys/netinet/in_pcb.h | 7 ++- sys/netinet/udp_usrreq.c | 11 ++-- sys/netinet6/icmp6.c | 14 +++--- sys/netinet6/in6_pcb.c | 11 ++-- sys/netinet6/in6_src.c | 100 ++++++++++++++++++------------------- sys/netinet6/ip6_var.h | 8 +-- sys/netinet6/nd6_nbr.c | 12 ++--- sys/netinet6/raw_ip6.c | 22 ++++---- sys/netinet6/udp6_output.c | 12 ++--- 10 files changed, 112 insertions(+), 129 deletions(-) diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index d9f1a20d391..7a08d386c81 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -1,4 +1,4 @@ -/* $OpenBSD: in_pcb.c,v 1.153 2014/04/16 13:04:38 mpi Exp $ */ +/* $OpenBSD: in_pcb.c,v 1.154 2014/04/18 10:48:29 jca Exp $ */ /* $NetBSD: in_pcb.c,v 1.25 1996/02/13 23:41:53 christos Exp $ */ /* @@ -388,13 +388,10 @@ in_pcbconnect(struct inpcb *inp, struct mbuf *nam) if (sin->sin_port == 0) return (EADDRNOTAVAIL); - ina = in_selectsrc(sin, inp->inp_moptions, &inp->inp_route, - &inp->inp_laddr, &error, inp->inp_rtableid); - if (ina == NULL) { - if (error == 0) - error = EADDRNOTAVAIL; + error = in_selectsrc(&ina, sin, inp->inp_moptions, &inp->inp_route, + &inp->inp_laddr, inp->inp_rtableid); + if (error) return (error); - } if (in_pcbhashlookup(inp->inp_table, sin->sin_addr, sin->sin_port, *ina, inp->inp_lport, inp->inp_rtableid) != 0) @@ -776,9 +773,10 @@ in_pcbrtentry(struct inpcb *inp) * If necessary, this function lookups the routing table and returns * an entry to the caller for later use. */ -struct in_addr * -in_selectsrc(struct sockaddr_in *sin, struct ip_moptions *mopts, - struct route *ro, struct in_addr *laddr, int *errorp, u_int rtableid) +int +in_selectsrc(struct in_addr **insrc, struct sockaddr_in *sin, + struct ip_moptions *mopts, struct route *ro, struct in_addr *laddr, + u_int rtableid) { struct sockaddr_in *sin2; struct in_ifaddr *ia = NULL; @@ -798,8 +796,10 @@ in_selectsrc(struct sockaddr_in *sin, struct ip_moptions *mopts, * If the source address is not specified but the socket(if any) * is already bound, use the bound address. */ - if (laddr && laddr->s_addr != INADDR_ANY) - return (laddr); + if (laddr && laddr->s_addr != INADDR_ANY) { + *insrc = laddr; + return (0); + } /* * If the destination address is multicast and an outgoing @@ -813,12 +813,11 @@ in_selectsrc(struct sockaddr_in *sin, struct ip_moptions *mopts, if (ifp != NULL) { if (ifp->if_rdomain == rtable_l2(rtableid)) IFP_TO_IA(ifp, ia); + if (ia == NULL) + return (EADDRNOTAVAIL); - if (ia == NULL) { - *errorp = EADDRNOTAVAIL; - return (NULL); - } - return (&ia->ia_addr.sin_addr); + *insrc = &ia->ia_addr.sin_addr; + return (0); } } /* @@ -851,15 +850,14 @@ in_selectsrc(struct sockaddr_in *sin, struct ip_moptions *mopts, */ if (ro->ro_rt && ro->ro_rt->rt_ifp) ia = ifatoia(ro->ro_rt->rt_ifa); - if (ia == 0) { + if (ia == NULL) { ia = TAILQ_FIRST(&in_ifaddr); - if (ia == 0) { - *errorp = EADDRNOTAVAIL; - return (NULL); - } + if (ia == NULL) + return (EADDRNOTAVAIL); } - return (&ia->ia_addr.sin_addr); + *insrc = &ia->ia_addr.sin_addr; + return (0); } void diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h index 29b933648d3..0c27f51c397 100644 --- a/sys/netinet/in_pcb.h +++ b/sys/netinet/in_pcb.h @@ -1,4 +1,4 @@ -/* $OpenBSD: in_pcb.h,v 1.84 2014/04/16 13:04:38 mpi Exp $ */ +/* $OpenBSD: in_pcb.h,v 1.85 2014/04/18 10:48:29 jca Exp $ */ /* $NetBSD: in_pcb.h,v 1.14 1996/02/13 23:42:00 christos Exp $ */ /* @@ -283,9 +283,8 @@ void in_rtchange(struct inpcb *, int); void in_setpeeraddr(struct inpcb *, struct mbuf *); void in_setsockaddr(struct inpcb *, struct mbuf *); int in_baddynamic(u_int16_t, u_int16_t); -struct in_addr * - in_selectsrc(struct sockaddr_in *, struct ip_moptions *, - struct route *, struct in_addr *, int *, u_int); +int in_selectsrc(struct in_addr **, struct sockaddr_in *, + struct ip_moptions *, struct route *, struct in_addr *, u_int); struct rtentry * in_pcbrtentry(struct inpcb *); diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index 4a90f786784..8fc4507e208 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -1,4 +1,4 @@ -/* $OpenBSD: udp_usrreq.c,v 1.181 2014/04/16 13:04:38 mpi Exp $ */ +/* $OpenBSD: udp_usrreq.c,v 1.182 2014/04/18 10:48:29 jca Exp $ */ /* $NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $ */ /* @@ -1017,13 +1017,10 @@ udp_output(struct mbuf *m, ...) goto release; } - laddr = in_selectsrc(sin, inp->inp_moptions, &inp->inp_route, - &inp->inp_laddr, &error, inp->inp_rtableid); - if (laddr == NULL) { - if (error == 0) - error = EADDRNOTAVAIL; + error = in_selectsrc(&laddr, sin, inp->inp_moptions, + &inp->inp_route, &inp->inp_laddr, inp->inp_rtableid); + if (error) goto release; - } if (inp->inp_lport == 0) { int s = splsoftnet(); diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c index 3d8a0f6ba78..a5b1f852738 100644 --- a/sys/netinet6/icmp6.c +++ b/sys/netinet6/icmp6.c @@ -1,4 +1,4 @@ -/* $OpenBSD: icmp6.c,v 1.142 2014/04/14 09:06:42 mpi Exp $ */ +/* $OpenBSD: icmp6.c,v 1.143 2014/04/18 10:48:30 jca Exp $ */ /* $KAME: icmp6.c,v 1.217 2001/06/20 15:03:29 jinmei Exp $ */ /* @@ -1962,7 +1962,7 @@ icmp6_reflect(struct mbuf *m, size_t off) struct ip6_hdr *ip6; struct icmp6_hdr *icmp6; struct in6_ifaddr *ia6; - struct in6_addr t, *src = 0; + struct in6_addr t, *src = NULL; int plen; int type, code; struct ifnet *outif = NULL; @@ -2060,8 +2060,8 @@ icmp6_reflect(struct mbuf *m, size_t off) src = &t; } - if (src == 0) { - int e; + if (src == NULL) { + int error; struct route_in6 ro; char addr[INET6_ADDRSTRLEN]; @@ -2071,18 +2071,18 @@ icmp6_reflect(struct mbuf *m, size_t off) * source address of the erroneous packet. */ bzero(&ro, sizeof(ro)); - src = in6_selectsrc(&sa6_src, NULL, NULL, &ro, NULL, &e, + error = in6_selectsrc(&src, &sa6_src, NULL, NULL, &ro, NULL, m->m_pkthdr.ph_rtableid); if (ro.ro_rt) { /* XXX: see comments in icmp6_mtudisc_update */ RTFREE(ro.ro_rt); /* XXX: we could use this */ } - if (src == NULL) { + if (error) { nd6log((LOG_DEBUG, "icmp6_reflect: source can't be determined: " "dst=%s, error=%d\n", inet_ntop(AF_INET6, &sa6_src.sin6_addr, addr, sizeof(addr)), - e)); + error)); goto bad; } } diff --git a/sys/netinet6/in6_pcb.c b/sys/netinet6/in6_pcb.c index 28e37dd3d0e..c3fd4790b76 100644 --- a/sys/netinet6/in6_pcb.c +++ b/sys/netinet6/in6_pcb.c @@ -1,4 +1,4 @@ -/* $OpenBSD: in6_pcb.c,v 1.61 2014/04/16 13:04:38 mpi Exp $ */ +/* $OpenBSD: in6_pcb.c,v 1.62 2014/04/18 10:48:30 jca Exp $ */ /* * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project. @@ -429,14 +429,11 @@ in6_pcbconnect(struct inpcb *inp, struct mbuf *nam) * with the address specified by setsockopt(IPV6_PKTINFO). * Is it the intended behavior? */ - in6a = in6_selectsrc(sin6, inp->inp_outputopts6, + error = in6_selectsrc(&in6a, sin6, inp->inp_outputopts6, inp->inp_moptions6, &inp->inp_route6, &inp->inp_laddr6, - &error, inp->inp_rtableid); - if (in6a == 0) { - if (error == 0) - error = EADDRNOTAVAIL; + inp->inp_rtableid); + if (error) return (error); - } if (inp->inp_route6.ro_rt && inp->inp_route6.ro_rt->rt_flags & RTF_UP) ifp = inp->inp_route6.ro_rt->rt_ifp; diff --git a/sys/netinet6/in6_src.c b/sys/netinet6/in6_src.c index 7ca882e4ab2..513aef2a245 100644 --- a/sys/netinet6/in6_src.c +++ b/sys/netinet6/in6_src.c @@ -1,4 +1,4 @@ -/* $OpenBSD: in6_src.c,v 1.41 2014/04/07 10:04:17 mpi Exp $ */ +/* $OpenBSD: in6_src.c,v 1.42 2014/04/18 10:48:30 jca Exp $ */ /* $KAME: in6_src.c,v 1.36 2001/02/06 04:08:17 itojun Exp $ */ /* @@ -96,18 +96,18 @@ int selectroute(struct sockaddr_in6 *, struct ip6_pktopts *, * If necessary, this function lookups the routing table and returns * an entry to the caller for later use. */ -struct in6_addr * -in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, - struct ip6_moptions *mopts, struct route_in6 *ro, struct in6_addr *laddr, - int *errorp, u_int rtableid) +int +in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock, + struct ip6_pktopts *opts, struct ip6_moptions *mopts, + struct route_in6 *ro, struct in6_addr *laddr, u_int rtableid) { struct ifnet *ifp = NULL; struct in6_addr *dst; struct in6_ifaddr *ia6 = NULL; struct in6_pktinfo *pi = NULL; + int error; dst = &dstsock->sin6_addr; - *errorp = 0; /* * If the source address is explicitly specified by the caller, @@ -120,9 +120,9 @@ in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, struct sockaddr_in6 sa6; /* get the outgoing interface */ - if ((*errorp = in6_selectif(dstsock, opts, mopts, ro, - &ifp, rtableid)) != 0) - return (NULL); + error = in6_selectif(dstsock, opts, mopts, ro, &ifp, rtableid); + if (error) + return (error); bzero(&sa6, sizeof(sa6)); sa6.sin6_family = AF_INET6; @@ -134,22 +134,23 @@ in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, ia6 = ifatoia6(ifa_ifwithaddr(sin6tosa(&sa6), rtableid)); if (ia6 == NULL || - (ia6->ia6_flags & (IN6_IFF_ANYCAST | IN6_IFF_NOTREADY))) { - *errorp = EADDRNOTAVAIL; - return (NULL); - } + (ia6->ia6_flags & (IN6_IFF_ANYCAST | IN6_IFF_NOTREADY))) + return (EADDRNOTAVAIL); pi->ipi6_addr = sa6.sin6_addr; /* XXX: this overrides pi */ - return (&pi->ipi6_addr); + *in6src = &pi->ipi6_addr; + return (0); } /* * If the source address is not specified but the socket(if any) * is already bound, use the bound address. */ - if (laddr && !IN6_IS_ADDR_UNSPECIFIED(laddr)) - return (laddr); + if (laddr && !IN6_IS_ADDR_UNSPECIFIED(laddr)) { + *in6src = laddr; + return (0); + } /* * If the caller doesn't specify the source address but @@ -158,16 +159,15 @@ in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, */ if (pi && pi->ipi6_ifindex) { ifp = if_get(pi->ipi6_ifindex); - if (ifp == NULL) { - *errorp = ENXIO; /* XXX: better error? */ - return (0); - } + if (ifp == NULL) + return (ENXIO); /* XXX: better error? */ + ia6 = in6_ifawithscope(ifp, dst, rtableid); - if (ia6 == 0) { - *errorp = EADDRNOTAVAIL; - return (0); - } - return (&ia6->ia_addr.sin6_addr); + if (ia6 == NULL) + return (EADDRNOTAVAIL); + + *in6src = &ia6->ia_addr.sin6_addr; + return (0); } /* @@ -182,16 +182,15 @@ in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, if ((IN6_IS_ADDR_LINKLOCAL(dst) || IN6_IS_ADDR_MC_LINKLOCAL(dst) || IN6_IS_ADDR_MC_INTFACELOCAL(dst)) && dstsock->sin6_scope_id) { ifp = if_get(dstsock->sin6_scope_id); - if (ifp == NULL) { - *errorp = ENXIO; /* XXX: better error? */ - return (0); - } + if (ifp == NULL) + return (ENXIO); /* XXX: better error? */ + ia6 = in6_ifawithscope(ifp, dst, rtableid); - if (ia6 == 0) { - *errorp = EADDRNOTAVAIL; - return (0); - } - return (&ia6->ia_addr.sin6_addr); + if (ia6 == NULL) + return (EADDRNOTAVAIL); + + *in6src = &ia6->ia_addr.sin6_addr; + return (0); } /* @@ -209,11 +208,11 @@ in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, if (ifp) { ia6 = in6_ifawithscope(ifp, dst, rtableid); - if (ia6 == 0) { - *errorp = EADDRNOTAVAIL; - return (0); - } - return (&ia6->ia_addr.sin6_addr); + if (ia6 == NULL) + return (EADDRNOTAVAIL); + + *in6src = &ia6->ia_addr.sin6_addr; + return (0); } } @@ -236,11 +235,11 @@ in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, if (ia6 == 0) ia6 = ifatoia6(rt->rt_ifa); } - if (ia6 == 0) { - *errorp = EADDRNOTAVAIL; - return (0); - } - return (&ia6->ia_addr.sin6_addr); + if (ia6 == NULL) + return (EADDRNOTAVAIL); + + *in6src = &ia6->ia_addr.sin6_addr; + return (0); } } @@ -286,15 +285,14 @@ in6_selectsrc(struct sockaddr_in6 *dstsock, struct ip6_pktopts *opts, if (ia6 == 0) /* xxx scope error ?*/ ia6 = ifatoia6(ro->ro_rt->rt_ifa); } - if (ia6 == 0) { - *errorp = EHOSTUNREACH; /* no route */ - return (0); - } - return (&ia6->ia_addr.sin6_addr); + if (ia6 == NULL) + return (EHOSTUNREACH); /* no route */ + + *in6src = &ia6->ia_addr.sin6_addr; + return (0); } - *errorp = EADDRNOTAVAIL; - return (0); + return (EADDRNOTAVAIL); } int diff --git a/sys/netinet6/ip6_var.h b/sys/netinet6/ip6_var.h index 04d43031e48..c78c161852e 100644 --- a/sys/netinet6/ip6_var.h +++ b/sys/netinet6/ip6_var.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ip6_var.h,v 1.47 2013/10/21 12:27:16 deraadt Exp $ */ +/* $OpenBSD: ip6_var.h,v 1.48 2014/04/18 10:48:30 jca Exp $ */ /* $KAME: ip6_var.h,v 1.33 2000/06/11 14:59:20 jinmei Exp $ */ /* @@ -308,9 +308,9 @@ int rip6_sysctl(int *, u_int, void *, size_t *, void *, size_t); int dest6_input(struct mbuf **, int *, int); int none_input(struct mbuf **, int *, int); -struct in6_addr *in6_selectsrc(struct sockaddr_in6 *, struct ip6_pktopts *, - struct ip6_moptions *, struct route_in6 *, struct in6_addr *, - int *, u_int); +int in6_selectsrc(struct in6_addr **, struct sockaddr_in6 *, + struct ip6_pktopts *, struct ip6_moptions *, struct route_in6 *, + struct in6_addr *, u_int); int in6_selectroute(struct sockaddr_in6 *, struct ip6_pktopts *, struct ip6_moptions *, struct route_in6 *, struct ifnet **, struct rtentry **, u_int rtableid); diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c index 52da768dcf2..729c959ea27 100644 --- a/sys/netinet6/nd6_nbr.c +++ b/sys/netinet6/nd6_nbr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: nd6_nbr.c,v 1.77 2014/04/14 09:06:42 mpi Exp $ */ +/* $OpenBSD: nd6_nbr.c,v 1.78 2014/04/18 10:48:30 jca Exp $ */ /* $KAME: nd6_nbr.c,v 1.61 2001/02/10 16:06:14 jinmei Exp $ */ /* @@ -456,9 +456,9 @@ nd6_ns_output(struct ifnet *ifp, struct in6_addr *daddr6, int error; bcopy(&dst_sa, &ro.ro_dst, sizeof(dst_sa)); - src0 = in6_selectsrc(&dst_sa, NULL, NULL, &ro, NULL, - &error, m->m_pkthdr.ph_rtableid); - if (src0 == NULL) { + error = in6_selectsrc(&src0, &dst_sa, NULL, NULL, &ro, + NULL, m->m_pkthdr.ph_rtableid); + if (error) { char addr[INET6_ADDRSTRLEN]; nd6log((LOG_DEBUG, @@ -968,9 +968,9 @@ nd6_na_output(struct ifnet *ifp, struct in6_addr *daddr6, * Select a source whose scope is the same as that of the dest. */ bcopy(&dst_sa, &ro.ro_dst, sizeof(dst_sa)); - src0 = in6_selectsrc(&dst_sa, NULL, NULL, &ro, NULL, &error, + error = in6_selectsrc(&src0, &dst_sa, NULL, NULL, &ro, NULL, m->m_pkthdr.ph_rtableid); - if (src0 == NULL) { + if (error) { char addr[INET6_ADDRSTRLEN]; nd6log((LOG_DEBUG, "nd6_na_output: source can't be " diff --git a/sys/netinet6/raw_ip6.c b/sys/netinet6/raw_ip6.c index 36ccd5afa5b..281ddc77ba9 100644 --- a/sys/netinet6/raw_ip6.c +++ b/sys/netinet6/raw_ip6.c @@ -1,4 +1,4 @@ -/* $OpenBSD: raw_ip6.c,v 1.65 2014/04/14 09:06:42 mpi Exp $ */ +/* $OpenBSD: raw_ip6.c,v 1.66 2014/04/18 10:48:30 jca Exp $ */ /* $KAME: raw_ip6.c,v 1.69 2001/03/04 15:55:44 itojun Exp $ */ /* @@ -413,13 +413,12 @@ rip6_output(struct mbuf *m, ...) { struct in6_addr *in6a; - if ((in6a = in6_selectsrc(dstsock, optp, in6p->inp_moptions6, - &in6p->inp_route6, &in6p->inp_laddr6, &error, - in6p->inp_rtableid)) == 0) { - if (error == 0) - error = EADDRNOTAVAIL; + error = in6_selectsrc(&in6a, dstsock, optp, + in6p->inp_moptions6, &in6p->inp_route6, &in6p->inp_laddr6, + in6p->inp_rtableid); + if (error) goto bad; - } + ip6->ip6_src = *in6a; if (in6p->inp_route6.ro_rt && in6p->inp_route6.ro_rt->rt_flags & RTF_UP) @@ -722,14 +721,11 @@ rip6_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam, } /* Source address selection. XXX: need pcblookup? */ - in6a = in6_selectsrc(addr, in6p->inp_outputopts6, + error = in6_selectsrc(&in6a, addr, in6p->inp_outputopts6, in6p->inp_moptions6, &in6p->inp_route6, - &in6p->inp_laddr6, &error, in6p->inp_rtableid); - if (in6a == NULL) { - if (error == 0) - error = EADDRNOTAVAIL; + &in6p->inp_laddr6, in6p->inp_rtableid); + if (error) break; - } in6p->inp_laddr6 = *in6a; in6p->inp_faddr6 = addr->sin6_addr; soisconnected(so); diff --git a/sys/netinet6/udp6_output.c b/sys/netinet6/udp6_output.c index be94e2c500c..4df8e16258e 100644 --- a/sys/netinet6/udp6_output.c +++ b/sys/netinet6/udp6_output.c @@ -1,4 +1,4 @@ -/* $OpenBSD: udp6_output.c,v 1.26 2014/04/16 13:04:38 mpi Exp $ */ +/* $OpenBSD: udp6_output.c,v 1.27 2014/04/18 10:48:30 jca Exp $ */ /* $KAME: udp6_output.c,v 1.21 2001/02/07 11:51:54 itojun Exp $ */ /* @@ -152,14 +152,12 @@ udp6_output(struct inpcb *in6p, struct mbuf *m, struct mbuf *addr6, } /* we don't support IPv4 mapped address */ - laddr = in6_selectsrc(sin6, optp, + error = in6_selectsrc(&laddr, sin6, optp, in6p->inp_moptions6, &in6p->inp_route6, - &in6p->inp_laddr6, &error, in6p->inp_rtableid); - if (laddr == NULL) { - if (error == 0) - error = EADDRNOTAVAIL; + &in6p->inp_laddr6, in6p->inp_rtableid); + if (error) goto release; - } + if (in6p->inp_lport == 0 && (error = in6_pcbsetport(laddr, in6p, p)) != 0) goto release; -- 2.20.1