On Oct 23, 2012, at 6:23 AM, Bruce Evans wrote:
> 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.
Hi Bruce,
please note that the SCTP code in the FreeBSD sources is generated
via an export script from a codebase which runs on multiple platforms.
The script tries to follow FreeBSDs guidelines, but is far from being
perfect.
Best regards
Michael
>
>> 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
>
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
