On Mon, Aug 08, 2022 at 03:48:57PM +0300, Vitaliy Makkoveev wrote:
> We use if (error != 0) or if (m == NULL) idioms in the network stack, so
> I used them too in the trivial places.
I nearly got them consistent in netinet/netinet6
$ grep 'if (error)' netinet*/* | wc
106 347 3398
$ grep 'if (error != 0)' netinet*/* | wc
5 25 182
> But here is another version of the split diff.
I like it.
> original diff moved only `pr_attach' and `pr_detach' and left
> `pr_usrreq' as is.
Both ways are fine.
> Also I like to hide ugly "(*so->so_proto->pr_usrreqs->pru_usrreq)" calls
> within corresponding pru_*() wrappers.
That makes code more readable. It helps to see which parameters
are relevant.
> So, for the first step, just move existing user requests handlers
> pointers.
These are two mechanical steps. pru_*() wrappers and pr_usrreqs.
Only one of them would be easier to review. But anyway, I looked
at both of them.
> @@ -2408,9 +2414,7 @@ const struct protosw routesw[] = {
> .pr_flags = PR_ATOMIC|PR_ADDR|PR_WANTRCVD,
> .pr_output = route_output,
> .pr_ctloutput = route_ctloutput,
> - .pr_usrreq = route_usrreq,
> - .pr_attach = route_attach,
> - .pr_detach = route_detach,
> + .pr_usrreqs = &route_usrreqs,
> .pr_init = route_prinit,
> .pr_sysctl = sysctl_rtable
All the others have a TAB after .pr_usrreqs.
> +static inline int
> +pru_attach(const struct protosw *prp, struct socket *so, int proto)
> +{
> + return (*prp->pr_usrreqs->pru_attach)(so, proto);
> +}
All callers have so->so_proto == prp. So you can avoid the prp
parameter and use so->so_proto instead.
> +static inline int
> +pru_sense(struct socket *so, struct stat *ub)
> +{
> + return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> + PRU_SENSE, (struct mbuf *)ub, NULL, NULL, curproc);
> +}
> +static inline int
> +pru_sockaddr(struct socket *so, struct mbuf *addr)
> +{
> + return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> + PRU_SOCKADDR, NULL, addr, NULL, curproc);
> +}
> +
> +static inline int
> +pru_peeraddr(struct socket *so, struct mbuf *addr)
> +{
> + return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> + PRU_PEERADDR, NULL, addr, NULL, curproc);
> +}
pru_sense, pru_sockaddr, pru_peeraddr used struct proc *p, you
changed it to curproc. Maybe they are always the same, but this
change should not be hidden in such a large diff.
> --- sys/sys/socketvar.h 15 Jul 2022 17:20:24 -0000 1.106
> +++ sys/sys/socketvar.h 8 Aug 2022 12:42:41 -0000
> @@ -32,6 +32,9 @@
> * @(#)socketvar.h 8.1 (Berkeley) 6/2/93
> */
>
> +#ifndef _SYS_SOCKETVAR_H_
> +#define _SYS_SOCKETVAR_H_
> +
> #include <sys/selinfo.h> /* for struct selinfo */
> #include <sys/queue.h>
> #include <sys/sigio.h> /* for struct sigio_ref
> */
> @@ -370,3 +373,5 @@ void sbcheck(struct socket *, struct soc
> #endif /* SOCKBUF_DEBUG */
>
> #endif /* _KERNEL */
> +
> +#endif /* _SYS_SOCKETVAR_H_ */
Is _SYS_SOCKETVAR_H_ related to this diff?
bluhm
