Hi Joakim,
On Thu, Sep 08, 2011 at 01:31:31PM +0200, Joakim Tjernlund wrote:
> > Baruch Siach <[email protected]> wrote on 2011/09/08 07:06:27:
> > ...
> >
> > > > if (nbytes & 1) /* Odd */
> > > > sum += *(u_char *)ptr; /* one byte only */
> > >
> > > This breaks big endian systems like PowerPC. The assignment via pointer
> > > dance
> > > of the original implementation is required, because we need the last byte
> > > to
> > > be considered as MSB of a 16bit word, as if we had one more '\0' byte in
> > > buffer. Note that the generated checksum is the same on big and little
> > > endian
> > > machines in terms of memory representation. They differ however in their
> > > numeric representation. So, for example the checksum of
> > >
> > > uint8_t buf[] = {0x12, 0x34, 0x56};
> > >
> > > is 0xcb97 on little endian machines, and 0x97cb on big endian machines.
> > > This
> > > is OK since we use the memory representation of the network packet.
> >
> > Ouch, even RFC 1071 got it wrong then.
>
> I just checked and all this ptr casting confuses gcc and generates
> bad code. Using an union is better, both for ppc and x86.
>
> This is only compile tested:
>
> if (len & 1) {
> union uu {
> uint16_t val;
> uint8_t b[2];
> } tmp = {0};
> /* Make sure that the left-over byte is added correctly both
> * with little and big endian hosts */
> tmp.b[0] = *(uint8_t*)w;
> sum += tmp.val;
> }
Run tested on x86_64 and PowerPC (with odd sized DHCP packets). However, I see
no significant code size improvement:
x86_64 : -4 bytes
ARM : +4 bytes
PowerPC: +16 bytes
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
