Skip site navigation (1)Skip section navigation (2)
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>