On Wed, Jan 08, 2014 at 12:02:25PM +0100, Martin Pieuchot wrote: > I find it really difficult to understand and work with the code of > rtsock.c because of the following defines: > > > /* Sleazy use of local variables throughout file, warning!!!! */ > #define dst info.rti_info[RTAX_DST] > #define gate info.rti_info[RTAX_GATEWAY] > ... > > But since this code is a mess, simply removing these defines makes it > harder to understand. So the diff below introduces other defines to > make it clear that we manipulate a rt_addrinfo structure while > preserving the readability: > > #define rti_dst rti_info[RTAX_DST] > #define rti_gate rti_info[RTAX_GATEWAY] > ... > > I converted rtsock.c and route.c and there's no object change with this > diff. I'll happily convert the remaining files after putting this in. > > Comments, ok? > > Index: net/route.c > =================================================================== > RCS file: /home/ncvs/src/sys/net/route.c,v > retrieving revision 1.147 > diff -u -p -r1.147 route.c > --- net/route.c 20 Oct 2013 13:21:57 -0000 1.147 > +++ net/route.c 8 Jan 2014 10:47:40 -0000 > @@ -325,7 +325,7 @@ rtalloc1(struct sockaddr *dst, int flags > int s = splsoftnet(), err = 0, msgtype = RTM_MISS; > > bzero(&info, sizeof(info)); > - info.rti_info[RTAX_DST] = dst; > + info.rti_dst = dst; > > rnh = rt_gettable(dst->sa_family, tableid); > if (rnh && (rn = rnh->rnh_matchaddr((caddr_t)dst, rnh)) && > @@ -346,13 +346,13 @@ rtalloc1(struct sockaddr *dst, int flags > } > /* Inform listeners of the new route */ > bzero(&info, sizeof(info)); > - info.rti_info[RTAX_DST] = rt_key(rt); > - info.rti_info[RTAX_NETMASK] = rt_mask(rt); > - info.rti_info[RTAX_GATEWAY] = rt->rt_gateway; > + info.rti_dst = rt_key(rt); > + info.rti_mask = rt_mask(rt); > + info.rti_gate = rt->rt_gateway;
I don't like this at all. Why remove the obvious rti_info[RTAX_DST] with a rti_dst macro? This makes to code even more obscure and harder to understand. So everything gets more confusing just so that rtsock.c can become a bit less fucked up. I think this is reverse and the crazy defines in rtsock should just be reversed. -- :wq Claudio
