Date: Tue, 23 Oct 2012 15:23:14 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Xin LI <delphij@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r241916 - in head/sys: netinet netinet6 Message-ID: <20121023142219.K1008@besplex.bde.org> In-Reply-To: <201210222149.q9MLnvrt014543@svn.freebsd.org> References: <201210222149.q9MLnvrt014543@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 22 Oct 2012, Xin LI wrote:
> Log:
> Remove __P.
This was a chance to remove style bugs in the prototypes. At least it
didn't create so many new ones, unlike the original __P axing. It
still enlarged about a hundred by changing from Gnu style continuation
to Gnu style continuation indentation with an off-by-5 error.
> Modified: head/sys/netinet/sctp_uio.h
> ==============================================================================
> --- head/sys/netinet/sctp_uio.h Mon Oct 22 21:26:36 2012 (r241915)
> +++ head/sys/netinet/sctp_uio.h Mon Oct 22 21:49:56 2012 (r241916)
> @@ -1267,44 +1267,44 @@ sctp_sorecvmsg(struct socket *so,
> #if !(defined(_KERNEL)) && !(defined(__Userspace__))
>
> __BEGIN_DECLS
> -int sctp_peeloff __P((int, sctp_assoc_t));
> -int sctp_bindx __P((int, struct sockaddr *, int, int));
> -int sctp_connectx __P((int, const struct sockaddr *, int, sctp_assoc_t *));
> -int sctp_getaddrlen __P((sa_family_t));
> -int sctp_getpaddrs __P((int, sctp_assoc_t, struct sockaddr **));
> -void sctp_freepaddrs __P((struct sockaddr *));
> -int sctp_getladdrs __P((int, sctp_assoc_t, struct sockaddr **));
> -void sctp_freeladdrs __P((struct sockaddr *));
> -int sctp_opt_info __P((int, sctp_assoc_t, int, void *, socklen_t *));
> +int sctp_peeloff(int, sctp_assoc_t);
> +int sctp_bindx(int, struct sockaddr *, int, int);
> +int sctp_connectx(int, const struct sockaddr *, int, sctp_assoc_t *);
> +int sctp_getaddrlen(sa_family_t);
> +int sctp_getpaddrs(int, sctp_assoc_t, struct sockaddr **);
> +void sctp_freepaddrs(struct sockaddr *);
> +int sctp_getladdrs(int, sctp_assoc_t, struct sockaddr **);
> +void sctp_freeladdrs(struct sockaddr *);
> +int sctp_opt_info(int, sctp_assoc_t, int, void *, socklen_t *);
sctp is fairly consistent in having style bugs on every line. It
never indented fucntion names in prototypes.
>
> /* deprecated */
> ssize_t sctp_sendmsg
> -__P((int, const void *, size_t, const struct sockaddr *,
> - socklen_t, uint32_t, uint32_t, uint16_t, uint32_t, uint32_t));
> +(int, const void *, size_t, const struct sockaddr *,
> + socklen_t, uint32_t, uint32_t, uint16_t, uint32_t, uint32_t);
Putting the __P(( unindented on a new line was weird. It is weirder
now with the __P( part.
>
> /* deprecated */
> - ssize_t sctp_send __P((int, const void *, size_t,
> - const struct sctp_sndrcvinfo *, int));
> + ssize_t sctp_send(int, const void *, size_t,
> + const struct sctp_sndrcvinfo *, int);
Continuation lines were weirdly indented in at least this part of sctp.
This one uses 14 spaces, underneath a line with 1 tab. 1 tab followed
by 4 spaces would be normal. Since the 14 spaces didn't line up with
anything, removing __P( leaves the continuation line not lined up with
anything slightly differently.
> ...
> /* deprecated */
> - ssize_t sctp_recvmsg __P((int, void *, size_t, struct sockaddr *, socklen_t *,
> - struct sctp_sndrcvinfo *, int *));
> + ssize_t sctp_recvmsg(int, void *, size_t, struct sockaddr *, socklen_t *,
> + struct sctp_sndrcvinfo *, int *);
Here there are 17 spaces instead of 14.
> Modified: head/sys/netinet6/icmp6.c
> ==============================================================================
> --- head/sys/netinet6/icmp6.c Mon Oct 22 21:26:36 2012 (r241915)
> +++ head/sys/netinet6/icmp6.c Mon Oct 22 21:49:56 2012 (r241916)
> @@ -133,15 +133,15 @@ VNET_DECLARE(int, icmp6_nodeinfo);
> static void icmp6_errcount(struct icmp6errstat *, int, int);
> static int icmp6_rip6_input(struct mbuf **, int);
> static int icmp6_ratelimit(const struct in6_addr *, const int, const int);
> -static const char *icmp6_redirect_diag __P((struct in6_addr *,
> - struct in6_addr *, struct in6_addr *));
> +static const char *icmp6_redirect_diag(struct in6_addr *,
> + struct in6_addr *, struct in6_addr *);
Function names not indented.
Continuation lines indented abnormally with 1 tab so that it doesn't
line up with anything.
> static struct mbuf *ni6_input(struct mbuf *, int);
> static struct mbuf *ni6_nametodns(const char *, int, int);
> static int ni6_dnsmatch(const char *, int, const char *, int);
> -static int ni6_addrs __P((struct icmp6_nodeinfo *, struct mbuf *,
> - struct ifnet **, struct in6_addr *));
> -static int ni6_store_addrs __P((struct icmp6_nodeinfo *, struct icmp6_nodeinfo *,
> - struct ifnet *, int));
Here the continuation lines were indented abnormally but using Gnu style
(indent -lp) so that they lined up with the parantheses using a mixture
of tabs and spaces.
> +static int ni6_addrs(struct icmp6_nodeinfo *, struct mbuf *,
> + struct ifnet **, struct in6_addr *);
> +static int ni6_store_addrs(struct icmp6_nodeinfo *, struct icmp6_nodeinfo *,
> + struct ifnet *, int);
Now after removing __P( and closing up a space, the contination lines don't
even follow Gnu style, but are off by 5 spaces.
> static int icmp6_notify_error(struct mbuf **, int, int, int);
>
> /*
>
> Modified: head/sys/netinet6/in6.c
> ==============================================================================
> --- head/sys/netinet6/in6.c Mon Oct 22 21:26:36 2012 (r241915)
> +++ head/sys/netinet6/in6.c Mon Oct 22 21:49:56 2012 (r241916)
> @@ -129,10 +129,10 @@ const struct in6_addr in6mask128 = IN6MA
> const struct sockaddr_in6 sa6_any =
> { sizeof(sa6_any), AF_INET6, 0, 0, IN6ADDR_ANY_INIT, 0 };
>
> -static int in6_lifaddr_ioctl __P((struct socket *, u_long, caddr_t,
> - struct ifnet *, struct thread *));
> -static int in6_ifinit __P((struct ifnet *, struct in6_ifaddr *,
> - struct sockaddr_in6 *, int));
> +static int in6_lifaddr_ioctl(struct socket *, u_long, caddr_t,
> + struct ifnet *, struct thread *);
> +static int in6_ifinit(struct ifnet *, struct in6_ifaddr *,
> + struct sockaddr_in6 *, int);
> static void in6_unlink_ifa(struct in6_ifaddr *, struct ifnet *);
>
> int (*faithprefix_p)(struct in6_addr *);
netinet6 mostly had the first type of misindentation of continued lines,
so removing __P( doesn't increase the style bugs.
> Modified: head/sys/netinet6/in6.h
> ==============================================================================
> --- head/sys/netinet6/in6.h Mon Oct 22 21:26:36 2012 (r241915)
> +++ head/sys/netinet6/in6.h Mon Oct 22 21:49:56 2012 (r241916)
> @@ -635,22 +635,22 @@ struct cmsghdr;
> struct ip6_hdr;
>
> int in6_cksum_pseudo(struct ip6_hdr *, uint32_t, uint8_t, uint16_t);
> -int in6_cksum __P((struct mbuf *, u_int8_t, u_int32_t, u_int32_t));
> -int in6_localaddr __P((struct in6_addr *));
> +int in6_cksum(struct mbuf *, u_int8_t, u_int32_t, u_int32_t);
> +int in6_localaddr(struct in6_addr *);
> int in6_localip(struct in6_addr *);
netinet actually had normal style here, except for __P( and a weird mixture
of uintN_t with u_intN_t.
> -int in6_addrscope __P((struct in6_addr *));
> -struct in6_ifaddr *in6_ifawithifp __P((struct ifnet *, struct in6_addr *));
> -extern void in6_if_up __P((struct ifnet *));
> +int in6_addrscope(struct in6_addr *);
> +struct in6_ifaddr *in6_ifawithifp(struct ifnet *, struct in6_addr *);
> +extern void in6_if_up(struct ifnet *);
'extern' on prototypes is even more archaic than __P(()), and is a style
bug. It may have been needed for portability in 1980.
> struct sockaddr;
Mispaced declaration:
(1) it is in the middle of the prototypes but not attached to the one (?)
prototype that uses it. For other declarations, this file uses the
more normal style of forward-incompete-declaring structs in a subsection
before all the prototypes.
(2) a subsection is not started or ended for it.
> extern u_char ip6_protox[];
>
Misplaced declaration:
(1) it is a data declaration in the middle of the prototypes but not even
used by the prototypes.
(2) a subsection is ended for it but not started.
> -void in6_sin6_2_sin __P((struct sockaddr_in *sin,
> - struct sockaddr_in6 *sin6));
> -void in6_sin_2_v4mapsin6 __P((struct sockaddr_in *sin,
> - struct sockaddr_in6 *sin6));
These used to line up.
> -void in6_sin6_2_sin_in_sock __P((struct sockaddr *nam));
> -void in6_sin_2_v4mapsin6_in_sock __P((struct sockaddr **nam));
> -extern void addrsel_policy_init __P((void));
Another extern, with no indentation.
> +void in6_sin6_2_sin(struct sockaddr_in *sin,
> + struct sockaddr_in6 *sin6);
> +void in6_sin_2_v4mapsin6(struct sockaddr_in *sin,
> + struct sockaddr_in6 *sin6);
Now off 5 spaces. The misindentation is more visible in the diffs than
above because both lines use tabs normally.
> +void in6_sin6_2_sin_in_sock(struct sockaddr *nam);
> +void in6_sin_2_v4mapsin6_in_sock(struct sockaddr **nam);
> +extern void addrsel_policy_init(void);
It still has the extern.
>
> #define satosin6(sa) ((struct sockaddr_in6 *)(sa))
> #define sin6tosa(sin6) ((struct sockaddr *)(sin6))
> @@ -674,43 +674,43 @@ typedef __socklen_t socklen_t;
> __BEGIN_DECLS
> struct cmsghdr;
>
> -extern int inet6_option_space __P((int));
> -extern int inet6_option_init __P((void *, struct cmsghdr **, int));
> -extern int inet6_option_append __P((struct cmsghdr *, const uint8_t *,
> - int, int));
> -extern uint8_t *inet6_option_alloc __P((struct cmsghdr *, int, int, int));
> -extern int inet6_option_next __P((const struct cmsghdr *, uint8_t **));
> -extern int inet6_option_find __P((const struct cmsghdr *, uint8_t **, int));
> -
> -extern size_t inet6_rthdr_space __P((int, int));
> -extern struct cmsghdr *inet6_rthdr_init __P((void *, int));
> -extern int inet6_rthdr_add __P((struct cmsghdr *, const struct in6_addr *,
> - unsigned int));
> -extern int inet6_rthdr_lasthop __P((struct cmsghdr *, unsigned int));
> +extern int inet6_option_space(int);
> +extern int inet6_option_init(void *, struct cmsghdr **, int);
> +extern int inet6_option_append(struct cmsghdr *, const uint8_t *,
> + int, int);
> +extern uint8_t *inet6_option_alloc(struct cmsghdr *, int, int, int);
> +extern int inet6_option_next(const struct cmsghdr *, uint8_t **);
> +extern int inet6_option_find(const struct cmsghdr *, uint8_t **, int);
> +
> +extern size_t inet6_rthdr_space(int, int);
> +extern struct cmsghdr *inet6_rthdr_init(void *, int);
> +extern int inet6_rthdr_add(struct cmsghdr *, const struct in6_addr *,
> + unsigned int);
> +extern int inet6_rthdr_lasthop(struct cmsghdr *, unsigned int);
> #if 0 /* not implemented yet */
> -extern int inet6_rthdr_reverse __P((const struct cmsghdr *, struct cmsghdr *));
> +extern int inet6_rthdr_reverse(const struct cmsghdr *, struct cmsghdr *);
> #endif
> -extern int inet6_rthdr_segments __P((const struct cmsghdr *));
> -extern struct in6_addr *inet6_rthdr_getaddr __P((struct cmsghdr *, int));
> -extern int inet6_rthdr_getflags __P((const struct cmsghdr *, int));
> -
> -extern int inet6_opt_init __P((void *, socklen_t));
> -extern int inet6_opt_append __P((void *, socklen_t, int, uint8_t, socklen_t,
> - uint8_t, void **));
> -extern int inet6_opt_finish __P((void *, socklen_t, int));
> -extern int inet6_opt_set_val __P((void *, int, void *, socklen_t));
> -
> -extern int inet6_opt_next __P((void *, socklen_t, int, uint8_t *, socklen_t *,
> - void **));
> -extern int inet6_opt_find __P((void *, socklen_t, int, uint8_t, socklen_t *,
> - void **));
> -extern int inet6_opt_get_val __P((void *, int, void *, socklen_t));
> -extern socklen_t inet6_rth_space __P((int, int));
> -extern void *inet6_rth_init __P((void *, socklen_t, int, int));
> -extern int inet6_rth_add __P((void *, const struct in6_addr *));
> -extern int inet6_rth_reverse __P((const void *, void *));
> -extern int inet6_rth_segments __P((const void *));
> -extern struct in6_addr *inet6_rth_getaddr __P((const void *, int));
> +extern int inet6_rthdr_segments(const struct cmsghdr *);
> +extern struct in6_addr *inet6_rthdr_getaddr(struct cmsghdr *, int);
> +extern int inet6_rthdr_getflags(const struct cmsghdr *, int);
> +
> +extern int inet6_opt_init(void *, socklen_t);
> +extern int inet6_opt_append(void *, socklen_t, int, uint8_t, socklen_t,
> + uint8_t, void **);
> +extern int inet6_opt_finish(void *, socklen_t, int);
> +extern int inet6_opt_set_val(void *, int, void *, socklen_t);
> +
> +extern int inet6_opt_next(void *, socklen_t, int, uint8_t *, socklen_t *,
> + void **);
> +extern int inet6_opt_find(void *, socklen_t, int, uint8_t, socklen_t *,
> + void **);
> +extern int inet6_opt_get_val(void *, int, void *, socklen_t);
> +extern socklen_t inet6_rth_space(int, int);
> +extern void *inet6_rth_init(void *, socklen_t, int, int);
> +extern int inet6_rth_add(void *, const struct in6_addr *);
> +extern int inet6_rth_reverse(const void *, void *);
> +extern int inet6_rth_segments(const void *);
> +extern struct in6_addr *inet6_rth_getaddr(const void *, int);
> __END_DECLS
Squillions of externs, and no tabs except to consistently misindent
continuation lines, so removing __P( didn't make the indentation worse.
This apparently uses extern because userland is more likely than the
kernel to be compiled by a 1980 model compiler. But the 1980 model
won't have dreamed of prototypes, so removing __P(()) breaks the
sources for it completely, and the externs are useless.
>
> #endif /* __BSD_VISIBLE */
>
> Modified: head/sys/netinet6/in6_gif.h
> ==============================================================================
> --- head/sys/netinet6/in6_gif.h Mon Oct 22 21:26:36 2012 (r241915)
> +++ head/sys/netinet6/in6_gif.h Mon Oct 22 21:49:56 2012 (r241916)
> @@ -36,10 +36,10 @@
> #define GIF_HLIM 30
>
> struct gif_softc;
> -int in6_gif_input __P((struct mbuf **, int *, int));
> -int in6_gif_output __P((struct ifnet *, int, struct mbuf *));
> -int gif_encapcheck6 __P((const struct mbuf *, int, int, void *));
> -int in6_gif_attach __P((struct gif_softc *));
> -int in6_gif_detach __P((struct gif_softc *));
> +int in6_gif_input(struct mbuf **, int *, int);
> +int in6_gif_output(struct ifnet *, int, struct mbuf *);
> +int gif_encapcheck6(const struct mbuf *, int, int, void *);
> +int in6_gif_attach(struct gif_softc *);
> +int in6_gif_detach(struct gif_softc *);
Another place for forward incomplete struct declarations.
These prototypes remain misindented and unsorted.
The gif_encapcheck6() one seems to be in a wrong namespace and is unsorted
into the middle of the in6 ones. If it were named in6_gig_enc..., then it
would still be unsorted.
>
> #endif /* _NETINET6_IN6_GIF_H_ */
>
Backwards comment.
[... Lots more similarly]
> Modified: head/sys/netinet6/in6_pcb.h
> ==============================================================================
> --- head/sys/netinet6/in6_pcb.h Mon Oct 22 21:26:36 2012 (r241915)
> +++ head/sys/netinet6/in6_pcb.h Mon Oct 22 21:49:56 2012 (r241916)
> @@ -72,53 +72,53 @@
> struct inpcbgroup *
> in6_pcbgroup_byhash(struct inpcbinfo *, u_int, uint32_t);
> struct inpcbgroup *
> - in6_pcbgroup_byinpcb __P((struct inpcb *));
> + in6_pcbgroup_byinpcb(struct inpcb *);
> struct inpcbgroup *
> in6_pcbgroup_bymbuf(struct inpcbinfo *, struct mbuf *);
> struct inpcbgroup *
> - in6_pcbgroup_bytuple __P((struct inpcbinfo *, const struct in6_addr *,
> - u_short, const struct in6_addr *, u_short));
> + in6_pcbgroup_bytuple(struct inpcbinfo *, const struct in6_addr *,
> + u_short, const struct in6_addr *, u_short);
Normal style to a fault. It even has the tab after 'struct' to bogusly
line up the struct tag. FreeBSD stopped requiring this mistake 10-15
years ago. I don't remember if it is still accepted.
With normal style, removing __P( doesn't affect the continued line.
This still uses u_short instead of u_int16_t or uint16_t.
> ...
> struct inpcb *
> - in6_pcblookup_local __P((struct inpcbinfo *,
> + in6_pcblookup_local(struct inpcbinfo *,
> struct in6_addr *, u_short, int,
> - struct ucred *));
> + struct ucred *);
Back to abnormal indentation for the continued lines.o
Removing __P( breaks it further, as usual.
The abnormal indentation is also partly responsible for needing 2
continuation lines instead of 1. But there was space for more args
before splitting, and after removal of __P( there is even more space.
Complete editing of __P( would create larger chur^diffs by often
moving 1 arg from each line back to the previous line in multi-line
prototypes after removing __P( gives enough space for 1 more arg on
the first line.
> struct inpcb *
> - in6_pcblookup __P((struct inpcbinfo *, struct in6_addr *,
> + in6_pcblookup(struct inpcbinfo *, struct in6_addr *,
> u_int, struct in6_addr *, u_int, int,
> - struct ifnet *));
> + struct ifnet *);
> struct inpcb *
> - in6_pcblookup_hash_locked __P((struct inpcbinfo *, struct in6_addr *,
> + in6_pcblookup_hash_locked(struct inpcbinfo *, struct in6_addr *,
> u_int, struct in6_addr *, u_int, int,
> - struct ifnet *));
> + struct ifnet *);
> struct inpcb *
> - in6_pcblookup_mbuf __P((struct inpcbinfo *, struct in6_addr *,
> + in6_pcblookup_mbuf(struct inpcbinfo *, struct in6_addr *,
> u_int, struct in6_addr *, u_int, int,
> - struct ifnet *ifp, struct mbuf *));
> -void in6_pcbnotify __P((struct inpcbinfo *, struct sockaddr *,
> + struct ifnet *ifp, struct mbuf *);
> +void in6_pcbnotify(struct inpcbinfo *, struct sockaddr *,
> u_int, const struct sockaddr *, u_int, int, void *,
> - struct inpcb *(*)(struct inpcb *, int)));
> + struct inpcb *(*)(struct inpcb *, int));
Lots more off-by-5 errors.
> struct inpcb *
> - in6_rtchange __P((struct inpcb *, int));
> + in6_rtchange(struct inpcb *, int);
After this point, most of the parameters have names, but before it they
don't.
> struct sockaddr *
> - in6_sockaddr __P((in_port_t port, struct in6_addr *addr_p));
> + in6_sockaddr(in_port_t port, struct in6_addr *addr_p);
> struct sockaddr *
> - in6_v4mapsin6_sockaddr __P((in_port_t port, struct in_addr *addr_p));
> -int in6_getpeeraddr __P((struct socket *so, struct sockaddr **nam));
> -int in6_getsockaddr __P((struct socket *so, struct sockaddr **nam));
> -int in6_mapped_sockaddr __P((struct socket *so, struct sockaddr **nam));
> -int in6_mapped_peeraddr __P((struct socket *so, struct sockaddr **nam));
> -int in6_selecthlim __P((struct in6pcb *, struct ifnet *));
> -int in6_pcbsetport __P((struct in6_addr *, struct inpcb *, struct ucred *));
Back to no names on parameters.
> -void init_sin6 __P((struct sockaddr_in6 *sin6, struct mbuf *m));
Especially large unsorting and/or namespace error.
> + in6_v4mapsin6_sockaddr(in_port_t port, struct in_addr *addr_p);
> +int in6_getpeeraddr(struct socket *so, struct sockaddr **nam);
> +int in6_getsockaddr(struct socket *so, struct sockaddr **nam);
> +int in6_mapped_sockaddr(struct socket *so, struct sockaddr **nam);
> +int in6_mapped_peeraddr(struct socket *so, struct sockaddr **nam);
> +int in6_selecthlim(struct in6pcb *, struct ifnet *);
> +int in6_pcbsetport(struct in6_addr *, struct inpcb *, struct ucred *);
> +void init_sin6(struct sockaddr_in6 *sin6, struct mbuf *m);
The order seems to have been originally alphabetical with random
accretions.
> #endif /* _KERNEL */
>
> #endif /* !_NETINET6_IN6_PCB_H_ */
At least this comment isn't backwards.
> ...
> Modified: head/sys/netinet6/in6_var.h
> ==============================================================================
> --- head/sys/netinet6/in6_var.h Mon Oct 22 21:26:36 2012 (r241915)
> +++ head/sys/netinet6/in6_var.h Mon Oct 22 21:49:56 2012 (r241916)
> @@ -762,36 +762,36 @@ int in6_leavegroup(struct in6_multi_mshi
> ...
> +int in6_prefix_ioctl(struct socket *, u_long, caddr_t,
> + struct ifnet *);
After removing __P(, the line splitting is no longer needed.
[... Lots more similarly]
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121023142219.K1008>