On Tue, Nov 16, 1999 at 03:17:43PM +1100, Bruce Evans wrote:
> On Mon, 15 Nov 1999, Pierre Beyssac wrote:
> > -   volatile u_short answer = 0;
> > +   union {
> > +       u_int16_t us;
> > +       u_int8_t  uc[2];
> > +   } answer;
> 
> This has indentation bugs.

Uh, which one(s) do you mean exactly? The 4-space indented union
(I just followed style(9)) or the double space before uc[2] (it
was just to align us and uc vertically)?

> ping.c still assumes that u_short is u_int16_t everywhere else.

But in_cksum() is more or less self-contained. Probably it's more
consistent (even withing in_cksum which uses u_short elsewhere) to
change back the union to u_short and u_char, though.

> This `answer' variable has nothing to do with the final `answer' variable.
> The latter should not be a union.  The original code apparently reuses
> `answer' to do manual register allocation for ancient compilers.

Agreed.

> Perhaps the above should be written as:
> 
>               sum += ntohs(*(u_char *)w << 8);
> 
> to avoid the undefined union access (answer.us).

Uh... I'm not sure I don't prefer the union, actually :-)
-- 
Pierre Beyssac          [EMAIL PROTECTED]


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to