Date: Sat, 14 Feb 2009 18:51:36 +0100 From: Luigi Rizzo <rizzo@iet.unipi.it> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, Luigi Rizzo <luigi@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r188578 - head/sys/netinet Message-ID: <20090214175136.GB89369@onelab2.iet.unipi.it> In-Reply-To: <20090214183758.I847@besplex.bde.org> References: <200902131514.n1DFEhft091837@svn.freebsd.org> <20090214183758.I847@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090214175136.GB89369>