svn commit: r188578 - head/sys/netinet

Luigi Rizzo rizzo at iet.unipi.it
Sat Feb 14 09:45:37 PST 2009


On Sat, Feb 14, 2009 at 06:59:57PM +1100, Bruce Evans wrote:
> 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

actually uid_t are uint32_t :)
But there are several differences:
- uid_t, dev_t and other widelyused_t are in <sys/types.h> so you don't
  need to add one extra header just for the n_long typedef;
- n_{long|short|time} are so rarely used that after 20 years one can
  safely declare they were an unsuccessful experiment;

> >+#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.

True. And this is another macro used exactly one time, which
probably could be retired or replaced with just the right number.

> >-			(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.

in fact, I prefer sizeof(variable) instead of sizeof(type)
in these circumstances, but both forms have pros and cons and i
just followed the existing form.

	cheers
	luigi


More information about the svn-src-head mailing list