Messages in this thread | | | From | David Matlack <> | Date | Sat, 17 May 2014 21:51:45 -0700 | Subject | Re: [PATCH 1/2] staging: slicoss: rewrite eeprom checksum code |
| |
On Sat, May 17, 2014 at 9:12 PM, Joe Perches <joe@perches.com> wrote: > On Sat, 2014-05-17 at 21:00 -0700, David Matlack wrote: > [] >> diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c > [] >> +static inline u16 __reduce(u32 checksum) >> +{ >> + u16 lower_16 = checksum & 0xFFFF; >> + u16 upper_16 = (checksum >> 16) & 0xFFFF; >> + >> + checksum = lower_16 + upper_16; >> + >> + if (checksum > 65535) >> + checksum -= 65535; >> + >> + return checksum; >> +} > > The if seems unnecessary. > > Perhaps declare a u16 return var or use > > return lower_16 + upper_16;
I agree it's fishy... but using overflow doesn't produce the same result:
(u16) 65536 == 0 65536 - 65535 == 1
Now which is the correct result, I have no idea. The eeprom on this device is small (0x80 bytes max, not enough to trigger overflow) and I have no documentation, so I don't know how to test :(
| |