Hi Joakim,
On Wed, Sep 07, 2011 at 09:05:36AM +0200, Joakim Tjernlund wrote:
> Baruch Siach <[email protected]> wrote on 2011/09/07 06:29:32:
> > Hi Joakim,
> > On Mon, Sep 05, 2011 at 10:09:14PM +0200, Joakim Tjernlund wrote:
> > > > From: Baruch Siach <[email protected]>
> > > >
> > > > Use a single IP checksum routine for ping, traceroute and udhcp.
> > > >
> > > > Signed-off-by: Baruch Siach <[email protected]>
> > > > ---
> > > >
> > > > Changes from v1:
> > > > Fix inet_cksum() with odd length on big endian systems
> > > > Remove declaration of removed udhcp_checksum()
> > >
> > > This looks like it is from quagga. I cleaned that up/optimized it
> > > some years ago into this(you might want to fix the types):
> >
> > What is the advantage of this implementation over the one I suggested
> > (which I
> > took from the traceroute code)?
>
> It is cleaner.
Which part of it looks cleaner to you? The two routine look almost the same to
me.
> Optimizes better(at least on RISC like archs).
I get the following 'readelf -s' results for different toolchains with '-Os':
x86_64 on current Debian testing (gcc 4.6.1):
8: 0000000000000000 78 FUNC GLOBAL DEFAULT 1 in_cksum
9: 000000000000004e 66 FUNC GLOBAL DEFAULT 1 inet_cksum
ARM with the CodeSourcery 2010q1 toolchain (gcc 4.4.1):
12: 00000000 100 FUNC GLOBAL DEFAULT 1 in_cksum
14: 00000064 104 FUNC GLOBAL DEFAULT 1 inet_cksum
PowerPC with the CodeSourcery 2010.09 toolchain (gcc 4.5.1):
8: 00000000 96 FUNC GLOBAL DEFAULT 1 in_cksum
9: 00000060 124 FUNC GLOBAL DEFAULT 1 inet_cksum
I don't see a definite winner here.
> I am not sure if this is allowed on modern gcc's:
> *(uint8_t*)&tmp = *(uint8_t*)w;
gcc with '-Wall -Wextra' doesn't complain.
> You can drop the pre increment if you like.
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
