lkml.org 
[lkml]   [2014]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 1/2] staging: slicoss: rewrite eeprom checksum code
On Mon, May 19, 2014 at 2:16 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> This patch is great, thanks, don't resend. But I wish the subject said
> "fix" instead of "rewrite". I was expecting it to just be a cleanup.
>
> It helps a lot if `git log --oneline` says which patches should be
> backported.
>
> regards,
> dan carpenter
>

Thanks for the review Dan. I know you said not resend but I'd like to clean up
the patch a bit if you don't mind:

* Add a comment making it clear this is RFC 1071

* Remove the byte swapping code. Specifically,

+ if ((unsigned long) eeprom & 1) {
+ leading_byte = bp;
+ bp++;
}

and

+ if (leading_byte) {
+ checksum = __reduce(checksum);
+ checksum <<= 8;
+
+ final_word = *leading_byte;
+ if (trailing_byte)
+ final_word |= *trailing_byte << 8;
+ }

After reading the RFC [1], I don't think the it's correct in either
implementation (the bytes in the final result are in the wrong order). And
since this function is only used to checksum the eeprom, and not checksum a
fragment of a IP packet, it's not needed. So we can just remove it.

* s/rewrite/fix/ in the subject

[1] http://tools.ietf.org/html/rfc1071 (see Section 2.(A) and 2.(B))


\
 
 \ /
  Last update: 2014-05-23 07:41    [W:0.071 / U:1.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site