Date: Fri, 10 Jan 2014 14:14:42 +0400 From: Gleb Smirnoff <glebius@FreeBSD.org> To: "Alexander V. Chernikov" <melifaro@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r260488 - head/sys/net Message-ID: <20140110101442.GB73147@FreeBSD.org> In-Reply-To: <201401091813.s09IDPlU058184@svn.freebsd.org> References: <201401091813.s09IDPlU058184@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Alexander,
some nitpicking:
On Thu, Jan 09, 2014 at 06:13:25PM +0000, Alexander V. Chernikov wrote:
A> @@ -52,6 +53,7 @@
A> #include <sys/proc.h>
A> #include <sys/domain.h>
A> #include <sys/kernel.h>
A> +#include <sys/kdb.h>
A>
A> #include <net/if.h>
A> #include <net/if_var.h>
A> @@ -86,6 +88,13 @@
A> #define RT_NUMFIBS 1
A> #endif
A>
A> +#if defined(INET) || defined(INET6)
A> +#ifdef SCTP
A> +extern void sctp_addr_change(struct ifaddr *ifa, int cmd);
A> +#endif /* SCTP */
A> +#endif
A> +
A> +
Can be simplified to one liner:
#if (defined(INET) || defined(INET6)) && defined(SCTP)
Same stands for same ifdef down below in code.
And extra empty line shouldn't have been added.
A> +
A> +/*
A> + * Announce interface address arrival/withdraw
A> + * Returns 0 on success.
A> + */
A> +int
A> +rt_addrmsg(int cmd, struct ifaddr *ifa, int fibnum)
A> +{
A> +
A> + KASSERT(cmd == RTM_ADD || cmd == RTM_DELETE,
A> + ("unexpected cmd %u", cmd));
A> +
A> + if (fibnum != RT_ALL_FIBS) {
A> + KASSERT(fibnum >= 0 && fibnum < rt_numfibs, ("%s: "
A> + "fibnum out of range 0 <= %d < %d", __func__,
A> + fibnum, rt_numfibs));
A> + }
A> +
A> + return (rtsock_addrmsg(cmd, ifa, fibnum));
A> +}
Second KASSERT together with if clause can be simplified to:
KASSERT(fibnum == RT_ALL_FIBS || (fibnum >= 0 && fibnum < rt_numfibs), ...
Same simplification can be done in rt_routemsg() and rt_newaddrmsg_fib().
A> +
A> +/*
A> + * Announce route addition/removal
A> + * Users of this function MUST validate input data BEFORE calling.
A> + * However we have to be able to handle invalid data:
A> + * if some userland app sends us "invalid" route message (invalid mask,
A> + * no dst, wrokg address families, etc...) we need to pass it back
^
typo
A> + * to app (and any other rtsock consumers) with rtm_errno field set to
A> + * non-zero value.
A> + * Returns 0 on success.
A> + */
A> +int
A> +rtsock_routemsg(int cmd, struct ifnet *ifp, int error, struct rtentry *rt,
A> + int fibnum)
A> {
A> + struct rt_addrinfo info;
A> + struct sockaddr *sa;
A> + struct mbuf *m;
A> + struct rt_msghdr *rtm;
A>
A> - rt_newaddrmsg_fib(cmd, ifa, error, rt, RT_ALL_FIBS);
A> + if (route_cb.any_count == 0)
A> + return (0);
A> +
A> + bzero((caddr_t)&info, sizeof(info));
A> + info.rti_info[RTAX_NETMASK] = rt_mask(rt);
A> + info.rti_info[RTAX_DST] = sa = rt_key(rt);
A> + info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
A> + if ((m = rt_msg1(cmd, &info)) == NULL)
A> + return (ENOBUFS);
A> + rtm = mtod(m, struct rt_msghdr *);
A> + rtm->rtm_index = ifp->if_index;
A> + rtm->rtm_flags |= rt->rt_flags;
A> + rtm->rtm_errno = error;
A> + rtm->rtm_addrs = info.rti_addrs;
A> +
A> + if (fibnum != RT_ALL_FIBS) {
A> + M_SETFIB(m, fibnum);
A> + m->m_flags |= RTS_FILTER_FIB;
A> + }
A> +
A> + rt_dispatch(m, sa ? sa->sa_family : AF_UNSPEC);
A> +
A> + return (0);
A> }
A>
A> +
Why extra line here?
A> /*
A> * This is the analogue to the rt_newaddrmsg which performs the same
A> * function but for multicast group memberhips. This is easier since
--
Totus tuus, Glebius.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140110101442.GB73147>