Re: [PATCH] staging: rtl8723au: Mark type casts to __le32 as intentional

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux