[U-Boot] [PATCH]Fix checksum to handle odd-length packet

[Greg Ren]

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.tjernlund at transmode.se] 
Sent: Thursday, December 03, 2009 6:41 AM
To: Wolfgang Denk
Cc: Greg Ren; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH]Fix checksum to handle odd-length packet

Wolfgang Denk <wd at denx.de> wrote on 03/12/2009 15:08:24:

> Dear Joakim Tjernlund,
>
> In message <OFDFF8A95F.374EB267-ONC1257681.0042613C-C1257681.
> 004485DD at 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: wd at denx.de
>        There is enough for the need of everyone in this world,
>        but not for the greed of everyone.     - Mahatma Gandhi
>