On Tue, Sep 22, 2015 at 11:30:54PM +0200, Larry Finger wrote: > > You may have silenced the Sparse warnings, but the code was not wrong. Your > version is also correct; however, you end up with some really ugly casts. > > Here is my analysis of these two, identical sections: > > The output of getcrc32() is an unsigned, 4-byte quantity that has the endianess > of the cpu. Therefore, the le32_to_cpu() conversion is suspect for a big-endian > machine. Those statements should be be a simple "actual_crc = getcrc32(....). > > The expected crc comes from a byte string that is in little-endian order. For > that reason, it needs to be converted on big-endian machines, which is exactly > what get_unaligned_le32() does. Thus, the second statement in each block becomes > "expected_crc = get_unaligned_le32(....)". > > Both the original code and your patch byte swap both quantities, thus they get > the correct result. at least if all you are doing is to compare the two results. I agree, it seems obvious when You spell it out like this. I was more focused on producing an identical result while removing the sparse warnings. Your suggestion is much nicer, I will change the patch accordingly. > > The above compiles with no Sparse warnings, and I think it would work on both LE > and BE architectures; however, it has only been compile tested. I have no big endian machine to try it on, but it seems reasonable to assume that at least the compare will work. The printk in case of a mismatch im not sure about, though. Assuming the analysis is correct, I think the printed values would be wrong on a BE machine before this change? Thanks, //Lars _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
