On Fri, 2012-09-28 at 11:41 -0500, Larry Finger wrote: > On 09/28/2012 04:04 AM, David Laight wrote: > >>> Index: wireless-testing-new/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c [] > >>> - *(u32 *)&rate_mask = (ratr_bitmap & 0x0fffffff) | > >>> - (ratr_index << 28); > >>> + for (i = 0; i < 3; i++) > >>> + rate_mask[i] = ratr_bitmap & (0xff << (i * 4)); [] > > So this either fixes, or adds, an endianness bug. > > Yes, the rate_mask array is little endian after this fragment is run, but the > only use of the byte array is to write it to the device, and LE is what it needs > no matter the platform. This change fixes an endianness bug. > > As I tend to get confused when doing these things, I wrote a small test program > and ran it on x86_64 and PPC-32 to confirm the result. > > Thanks for teaching me about a = b >>= 8. I was not aware that C could do that. The other thing that could be done is to use cpu_to_le32() but David's method I think is superior for this use as there's no absolute guarantee that rate_mask is aligned on a u32. I'd add parens to make the precedence explicit. rate_mask[0] = ratr_bitmap; rate_mask[1] = (ratr_bitmap >>= 8); rate_mask[2] = (ratr_bitmap >>= 8); rate_mask[3] = ((ratr_bitmap >> 8) & 0xf) | (ratr_index << 4); -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
