Thanks. It was not a clean solution as I only experimented on our processor which is big-endian.
The fact is that the original code is not endianess proof. It was coded for big-endian processors. Greg Ren -----Original Message----- From: Joakim Tjernlund [mailto:joakim.tjernl...@transmode.se] Sent: Thursday, December 03, 2009 6:41 AM To: Wolfgang Denk Cc: Greg Ren; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH]Fix checksum to handle odd-length packet Wolfgang Denk <w...@denx.de> wrote on 03/12/2009 15:08:24: > Dear Joakim Tjernlund, > > In message <OFDFF8A95F.374EB267-ONC1257681.0042613C-C1257681. > 00448...@transmode.se> you wrote: > > > > > > + if (len == 1) { > > > > + xsum += (*p & 0xff00); > > > > > > I doubt that this code is endianess-clean. > > > > Nope, I would think some thing like this would work better: > > count = len >> 1; /* div by 2 */ > > for(p--; count; --count) > > xsum += *++p; > > > > if (len & 1) /* Odd */ > > xsum += *(u_char *)(++p); /* one byte only */ > > It probably depends on the definition of "better" ;-) > > Running this test code: > > ----------------------------------------- > #include <stdio.h> > #include <string.h> > > #define LENGTH 6 > > int main (void) > { > char string[LENGTH] = { 1, 2, 3, 4, 5, 0, }; > unsigned short array[LENGTH/2]; > unsigned short *p; > unsigned short xsum, xsum1, xsum2; ulong, not short :) > int i, count; > > memcpy (array, string, LENGTH); > > count = LENGTH / 2; > > xsum = 0; > p = array; > > while (count > 1) { > printf ("Adding 0x%04x\n", *p); > xsum += *p++; > --count; > } for(p--; count; --count) xsum += *++p; if (LENGTH & 1) /* Odd */ xsum += *(unsigned char *)(++p); /* one byte only */ > > xsum1 = xsum + (*p & 0xff00); ehh, this has to go. > > xsum2 = xsum + *(unsigned char *)(p); You are not folding the sum, unsure if that has any significans > > printf ("*p = 0x%04x\n", *p); > printf ("xsum = %04x, xsum1 = %04x, xsum2 = %04x\n", > xsum, xsum1, xsum2); > > return (0); > } > ----------------------------------------- > > on a little endian system gives: > > -> ./f-le > Adding 0x0201 > Adding 0x0403 > *p = 0x0005 > xsum = 0604, xsum1 = 0604, xsum2 = 0609 > > while running it on a big endian system gives > > -> ./f-be > Adding 0x0102 > Adding 0x0304 > *p = 0x0500 > xsum = 0406, xsum1 = 0906, xsum2 = 040b > > I don't know what you consider to be the exact result, but fact is > that even if we ignore byte swapping, none of the implementations is > endianess clean. > > Of course, there is a chance that I mis-implemented your suggestion. > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de > There is enough for the need of everyone in this world, > but not for the greed of everyone. - Mahatma Gandhi > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot