On Mon, 15 Nov 1999, Pierre Beyssac wrote:
> On Mon, Nov 15, 1999 at 01:35:15PM -0500, Garrett Wollman wrote:
> > If, rather than casting pointers, the code used a union (containing
> > one u_int16_t and one array[2] of u_int8_t), the compiler would have
> > enough information to know about the aliases.
>
> You're right, this seems to work even with optimization turned on.
> If nobody objects, I'll commit it.
>
> --- ck.c.old Mon Nov 15 19:41:35 1999
> +++ ck.c Mon Nov 15 19:39:43 1999
> @@ -13,7 +13,10 @@
> register int nleft = len;
> register u_short *w = addr;
> int sum = 0;
> - volatile u_short answer = 0;
> + union {
> + u_int16_t us;
> + u_int8_t uc[2];
> + } answer;
This has indentation bugs.
ping.c still assumes that u_short is u_int16_t everywhere else.
>
> /*
> * Our algorithm is simple, using a 32 bit accumulator (sum), we add
> @@ -27,15 +30,16 @@
>
> /* mop up an odd byte, if necessary */
> if (nleft == 1) {
> - *(u_char *)(&answer) = *(u_char *)w ;
> - sum += answer;
> + answer.uc[0] = *(u_char *)w ;
> + answer.uc[1] = 0;
> + sum += answer.us;
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.
Perhaps the above should be written as:
sum += ntohs(*(u_char *)w << 8);
to avoid the undefined union access (answer.us). I think this works
on all systems, but it is a pessimisation on some little-endian systems
including i386's (on i386's, ntohs() is inline, but it is inline asm
so the compiler can't see that it just reverses the shift).
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
