rtsock.c: eliminate masking of gotos, don't abuse M_RTABLE
Bruce M Simpson
bms at spc.org
Fri Oct 3 00:00:41 PDT 2003
Hi,
Here's a diff to eliminate the senderr() macro from rtsock.c.
This macro is masking goto statements, which is incredibly bad style,
and makes it difficult to follow the flow of control in the file.
This diff also stops rtsock.c from abusing the M_RTABLE malloc define
for routing socket messages, for the sake of being clearer.
[I would have liked to keep these separate but I haven't setup a local
branch yet.]
The size of an rtmsg can vary, but should perhaps be constant --
there are bugs present, as exercised by some code I posted to this list,
and to ru@ in private, just over a month or so ago which panics the kernel
by passing in a dubiously-formatted PF_ROUTE message.
Making rtmsg constant and no longer using the packed sockaddr format would
make it a candiate for a zone allocator, and would help eliminate some
redundancy between rtsock.c and ifconfig(8)/route(8) in the userland.
Comments?
BMS
-------------- next part --------------
Index: rtsock.c
===================================================================
RCS file: /home/ncvs/src/sys/net/rtsock.c,v
retrieving revision 1.89
diff -u -r1.89 rtsock.c
--- rtsock.c 5 Mar 2003 19:24:22 -0000 1.89
+++ rtsock.c 3 Oct 2003 06:45:18 -0000
@@ -55,6 +55,14 @@
MALLOC_DEFINE(M_RTABLE, "routetbl", "routing tables");
+MALLOC_DEFINE(M_RTMSG, "rtmsg", "PF_ROUTE message data");
+
+#define RTMSG_MALLOC(p, n) \
+ ((p) = (struct rt_msghdr *) \
+ malloc((unsigned long)(n), M_RTMSG, M_NOWAIT))
+#define RTMSG_FREE(p) \
+ free((caddr_t)(p), M_RTMSG)
+
static struct sockaddr route_dst = { 2, PF_ROUTE, };
static struct sockaddr route_src = { 2, PF_ROUTE, };
static struct sockaddr sa_zero = { sizeof(sa_zero), AF_INET, };
@@ -280,7 +288,6 @@
struct ifnet *ifp = 0;
struct ifaddr *ifa = 0;
-#define senderr(e) { error = e; goto flush;}
if (m == 0 || ((m->m_len < sizeof(long)) &&
(m = m_pullup(m, sizeof(long))) == 0))
return (ENOBUFS);
@@ -290,37 +297,45 @@
if (len < sizeof(*rtm) ||
len != mtod(m, struct rt_msghdr *)->rtm_msglen) {
dst = 0;
- senderr(EINVAL);
+ error = EINVAL;
+ goto flush;
}
- R_Malloc(rtm, struct rt_msghdr *, len);
+ RTMSG_MALLOC(rtm, len);
if (rtm == 0) {
dst = 0;
- senderr(ENOBUFS);
+ error = ENOBUFS;
+ goto flush;
}
m_copydata(m, 0, len, (caddr_t)rtm);
if (rtm->rtm_version != RTM_VERSION) {
dst = 0;
- senderr(EPROTONOSUPPORT);
+ error = EPROTONOSUPPORT;
+ goto flush;
}
rtm->rtm_pid = curproc->p_pid;
bzero(&info, sizeof(info));
info.rti_addrs = rtm->rtm_addrs;
if (rt_xaddrs((caddr_t)(rtm + 1), len + (caddr_t)rtm, &info)) {
dst = 0;
- senderr(EINVAL);
+ error = EINVAL;
+ goto flush;
}
info.rti_flags = rtm->rtm_flags;
if (dst == 0 || (dst->sa_family >= AF_MAX)
- || (gate != 0 && (gate->sa_family >= AF_MAX)))
- senderr(EINVAL);
+ || (gate != 0 && (gate->sa_family >= AF_MAX))) {
+ error = EINVAL;
+ goto flush;
+ }
if (genmask) {
struct radix_node *t;
t = rn_addmask((caddr_t)genmask, 0, 1);
if (t && Bcmp((caddr_t *)genmask + 1, (caddr_t *)t->rn_key + 1,
*(u_char *)t->rn_key - 1) == 0)
genmask = (struct sockaddr *)(t->rn_key);
- else
- senderr(ENOBUFS);
+ else {
+ error = ENOBUFS;
+ goto flush;
+ }
}
/*
@@ -328,13 +343,15 @@
* is the only operation the non-superuser is allowed.
*/
if (rtm->rtm_type != RTM_GET && (error = suser(curthread)) != 0)
- senderr(error);
+ goto flush;
switch (rtm->rtm_type) {
case RTM_ADD:
- if (gate == 0)
- senderr(EINVAL);
+ if (gate == 0) {
+ error = EINVAL;
+ goto flush;
+ }
error = rtrequest1(RTM_ADD, &info, &saved_nrt);
if (error == 0 && saved_nrt) {
rt_setmetrics(rtm->rtm_inits,
@@ -359,15 +376,18 @@
case RTM_CHANGE:
case RTM_LOCK:
if ((rnh = rt_tables[dst->sa_family]) == 0) {
- senderr(EAFNOSUPPORT);
+ error = EAFNOSUPPORT;
+ goto flush;
}
RADIX_NODE_HEAD_LOCK(rnh);
rt = (struct rtentry *) rnh->rnh_lookup(dst, netmask, rnh);
RADIX_NODE_HEAD_UNLOCK(rnh);
if (rt != NULL)
rt->rt_refcnt++;
- else
- senderr(ESRCH);
+ else {
+ error = ESRCH;
+ goto flush;
+ }
switch(rtm->rtm_type) {
@@ -394,11 +414,13 @@
(struct walkarg *)0);
if (len > rtm->rtm_msglen) {
struct rt_msghdr *new_rtm;
- R_Malloc(new_rtm, struct rt_msghdr *, len);
- if (new_rtm == 0)
- senderr(ENOBUFS);
+ RTMSG_MALLOC(new_rtm, len);
+ if (new_rtm == 0) {
+ error = ENOBUFS;
+ goto flush;
+ }
Bcopy(rtm, new_rtm, rtm->rtm_msglen);
- Free(rtm); rtm = new_rtm;
+ RTMSG_FREE(rtm); rtm = new_rtm;
}
(void)rt_msg2(rtm->rtm_type, &info, (caddr_t)rtm,
(struct walkarg *)0);
@@ -418,11 +440,11 @@
(ifaaddr != NULL &&
!sa_equal(ifaaddr, rt->rt_ifa->ifa_addr))) {
if ((error = rt_getifa(&info)) != 0)
- senderr(error);
+ goto flush;
}
if (gate != NULL &&
(error = rt_setgate(rt, rt_key(rt), gate)) != 0)
- senderr(error);
+ goto flush;
if ((ifa = info.rti_ifa) != NULL) {
register struct ifaddr *oifa = rt->rt_ifa;
if (oifa != ifa) {
@@ -453,7 +475,7 @@
break;
default:
- senderr(EOPNOTSUPP);
+ error = EOPNOTSUPP;
}
flush:
@@ -473,7 +495,7 @@
if ((so->so_options & SO_USELOOPBACK) == 0) {
if (route_cb.any_count <= 1) {
if (rtm)
- Free(rtm);
+ RTMSG_FREE(rtm);
m_freem(m);
return (error);
}
@@ -487,7 +509,7 @@
m = NULL;
} else if (m->m_pkthdr.len > rtm->rtm_msglen)
m_adj(m, rtm->rtm_msglen - m->m_pkthdr.len);
- Free(rtm);
+ RTMSG_FREE(rtm);
}
if (rp)
rp->rcb_proto.sp_family = 0; /* Avoid us */
More information about the freebsd-net
mailing list