svn commit: r241916 - in head/sys: netinet netinet6

Bruce Evans brde at optusnet.com.au
Tue Oct 23 04:23:30 UTC 2012 

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

More information about the svn-src-all mailing list