svn commit: r188578 - head/sys/netinet

[Bruce Evans]

On Fri, 13 Feb 2009, Luigi Rizzo wrote:

> Log:
>  Use uint32_t instead of n_long and n_time, and uint16_t instead of n_short.
>  Add a note next to fields in network format.
>
>  The n_* types are not enough for compiler checks on endianness, and their
>  use often requires an otherwise unnecessary #include <netinet/in_systm.h>

This is too much like globally substituting uid_t with uint16_t.  At
least the n_time typedef provides some correct opaqueness (while n_long
is just bogus since the network size is 4 octets and not 1 long on
64-bit machines).

> Modified: head/sys/netinet/ip.h
> ==============================================================================
> --- head/sys/netinet/ip.h	Fri Feb 13 14:43:46 2009	(r188577)
> +++ head/sys/netinet/ip.h	Fri Feb 13 15:14:43 2009	(r188578)
> @@ -150,10 +150,10 @@ struct	ip_timestamp {
> 		ipt_flg:4;		/* flags, see below */
> #endif
> 	union ipt_timestamp {
> -		n_long	ipt_time[1];
> +		uint32_t	ipt_time[1];	/* network format */

The old typedef-names also allow better formatting (since they are shorter
than 8 characters).  This declaration is now indented an extra 8 characters...

> 		struct	ipt_ta {
> 			struct in_addr ipt_addr;
> -			n_long ipt_time;
> +			uint32_t ipt_time;	/* network format */

... while this one is indented inconsistently by 1 space before and after.
Similarly elsewhere.

The new comments are bogus.  Surely everything here is in network format,
not just the now-annotated fields.  Similarly elsewhere.

> Modified: head/sys/netinet/ip_icmp.h
> ==============================================================================
> --- head/sys/netinet/ip_icmp.h	Fri Feb 13 14:43:46 2009	(r188577)
> +++ head/sys/netinet/ip_icmp.h	Fri Feb 13 15:14:43 2009	(r188578)
> @@ -127,7 +131,7 @@ struct icmp {
>  * ip header length.
>  */
> #define	ICMP_MINLEN	8				/* abs minimum */
> -#define	ICMP_TSLEN	(8 + 3 * sizeof (n_time))	/* timestamp */
> +#define	ICMP_TSLEN	(8 + 3 * sizeof (uint32_t))	/* timestamp */

The changes lose the most where they involve sizeof's.  Now it is
unclear that the sizeof is of 1 timestamp.  sizeof(uint32_t) is an
obfuscated spelling of 4.

This change preserves the style bug.  sizeof's are not followed by a space.

> Modified: head/sys/netinet/ip_options.c
> ==============================================================================
> --- head/sys/netinet/ip_options.c	Fri Feb 13 14:43:46 2009	(r188577)
> +++ head/sys/netinet/ip_options.c	Fri Feb 13 15:14:43 2009	(r188578)
> @@ -105,7 +105,7 @@ ip_dooptions(struct mbuf *m, int pass)
> 	struct in_ifaddr *ia;
> 	int opt, optlen, cnt, off, code, type = ICMP_PARAMPROB, forward = 0;
> 	struct in_addr *sin, dst;
> -	n_time ntime;
> +	uint32_t ntime;
> 	struct	sockaddr_in ipaddr = { sizeof(ipaddr), AF_INET };
>
> 	/* Ignore or reject packets with IP options. */
> @@ -320,7 +320,7 @@ dropit:
> 				break;
>
> 			case IPOPT_TS_TSANDADDR:
> -				if (off + sizeof(n_time) +
> +				if (off + sizeof(uint32_t) +
> 				    sizeof(struct in_addr) > optlen) {
> 					code = &cp[IPOPT_OFFSET] - (u_char *)ip;
> 					goto bad;
> @@ -337,7 +337,7 @@ dropit:
> 				break;
>
> 			case IPOPT_TS_PRESPEC:
> -				if (off + sizeof(n_time) +
> +				if (off + sizeof(uint32_t) +
> 				    sizeof(struct in_addr) > optlen) {
> 					code = &cp[IPOPT_OFFSET] - (u_char *)ip;
> 					goto bad;

sizeof(type) is probably the best spelling of the magic number in the above...

> @@ -355,8 +355,8 @@ dropit:
> 				goto bad;
> 			}
> 			ntime = iptime();
> -			(void)memcpy(cp + off, &ntime, sizeof(n_time));
> -			cp[IPOPT_OFFSET] += sizeof(n_time);
> +			(void)memcpy(cp + off, &ntime, sizeof(uint32_t));
> +			cp[IPOPT_OFFSET] += sizeof(uint32_t);

... but here we are copying the object `ntime'; sizeof(n_time) was an
obfuscated spelling of the size of that object, and sizeof(uint32_t) is
an even more obfuscated spelling.  Similarly elsewhere.

Bruce