On Mon, Aug 10, 2015 at 11:06:52PM +0200, Martin Pieuchot wrote:
> Convert two rt->rt_refcnt-- to rtfree(rt). In both cases the entry
> is returned from rtrequest1(RTM_ADD, ...) and wont be freed because
> it currently *is* in the table.
>
> Ok?
You changed from using nrt to rt. Both variables are used only in
the if block and there they have the same value. Please consider
removing nrt from rt_ifa_add(). The end of rt_ifa_del() has a
similar rt = nrt, both functions should use the same variable names.
Currently we have the pattern to do rt->rt_refcnt-- instead of
rtfree(rt) in some places. This is valid if it is guaranteed that
more than one reference exists. Do you suggest to replace that
with rtfree(rt) in general?
In the two cases here, rtfree(rt) makes sense as the current reference
is actually used after decrementing its counter. Better free it
after using.
OK bluhm@
>
> Index: net/route.c
> ===================================================================
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.217
> diff -u -p -r1.217 route.c
> --- net/route.c 18 Jul 2015 15:51:16 -0000 1.217
> +++ net/route.c 10 Aug 2015 21:04:11 -0000
> @@ -1181,7 +1181,6 @@ rt_ifa_add(struct ifaddr *ifa, int flags
>
> error = rtrequest1(RTM_ADD, &info, prio, &nrt, rtableid);
> if (error == 0 && (rt = nrt) != NULL) {
> - rt->rt_refcnt--;
> if (rt->rt_ifa != ifa) {
> printf("%s: wrong ifa (%p) was (%p)\n", __func__,
> ifa, rt->rt_ifa);
> @@ -1201,8 +1200,9 @@ rt_ifa_add(struct ifaddr *ifa, int flags
> * userland that a new address has been added.
> */
> if (flags & RTF_LOCAL)
> - rt_sendaddrmsg(nrt, RTM_NEWADDR);
> - rt_sendmsg(nrt, RTM_ADD, rtableid);
> + rt_sendaddrmsg(rt, RTM_NEWADDR);
> + rt_sendmsg(rt, RTM_ADD, rtableid);
> + rtfree(rt);
> }
> return (error);
> }
> Index: net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.166
> diff -u -p -r1.166 rtsock.c
> --- net/rtsock.c 18 Jul 2015 21:58:06 -0000 1.166
> +++ net/rtsock.c 10 Aug 2015 21:00:58 -0000
> @@ -598,11 +598,11 @@ route_output(struct mbuf *m, ...)
> if (error == 0 && saved_nrt) {
> rt_setmetrics(rtm->rtm_inits, &rtm->rtm_rmx,
> &saved_nrt->rt_rmx);
> - saved_nrt->rt_refcnt--;
> /* write back the priority the kernel used */
> rtm->rtm_priority = saved_nrt->rt_priority & RTP_MASK;
> rtm->rtm_index = saved_nrt->rt_ifp->if_index;
> rtm->rtm_flags = saved_nrt->rt_flags;
> + rtfree(saved_nrt);
> }
> break;
> case RTM_DELETE:
