From owner-svn-src-all@FreeBSD.ORG Fri Jan 10 12:20:32 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 7DA918BE; Fri, 10 Jan 2014 12:20:32 +0000 (UTC) Received: from mail.ipfw.ru (mail.ipfw.ru [IPv6:2a01:4f8:120:6141::2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 3F47A1718; Fri, 10 Jan 2014 12:20:32 +0000 (UTC) Received: from [2a02:6b8:0:401:222:4dff:fe50:cd2f] (helo=ptichko.yndx.net) by mail.ipfw.ru with esmtpsa (TLSv1:CAMELLIA256-SHA:256) (Exim 4.76 (FreeBSD)) (envelope-from ) id 1W1XF6-000Nk6-V4; Fri, 10 Jan 2014 12:15:01 +0400 Message-ID: <52CFE4D4.8080700@FreeBSD.org> Date: Fri, 10 Jan 2014 16:17:24 +0400 From: "Alexander V. Chernikov" User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.0.1 MIME-Version: 1.0 To: Gleb Smirnoff Subject: Re: svn commit: r260488 - head/sys/net References: <201401091813.s09IDPlU058184@svn.freebsd.org> <20140110101442.GB73147@FreeBSD.org> In-Reply-To: <20140110101442.GB73147@FreeBSD.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Jan 2014 12:20:32 -0000 On 10.01.2014 14:14, Gleb Smirnoff wrote: > Alexander, > > some nitpicking: > > On Thu, Jan 09, 2014 at 06:13:25PM +0000, Alexander V. Chernikov wrote: > A> @@ -52,6 +53,7 @@ > A> #include > A> #include > A> #include > A> +#include > A> > A> #include > A> #include > 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. This actually needs to be removed from here and converted to be ifaddr_event consumer. > > 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(). Yes, thanks. > > 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 Fixed. > > 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? Fixed. > > 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 >