On Sun, Oct 3, 2010 at 4:10 PM, Peter Stuge peter@stuge.se wrote:
Rudolf just found a bug in the sb700 code:
u32 dword; .. dword = pci_read_config8(dev, 0x64); dword |= 1 << 10; pci_write_config8(dev, 0x64, dword);
Actually, I don't even have a problem with this construct. Why? Because it's in just about every kernel I've ever worked with. It's a common technique. Sure, in this case, it's a trivially simple change and you can write a function for it. But, as soon as things get more complex, with multiline tests and bit sets, you can't use the functions, and we're back to the same type of coding. Then we end up with mixed idioms.
It's a good idea for our code base to adhere to such common idioms. It makes for an easier time for people coming in from, e.g., Linux. I don't find the functions easier.
Also, as pointed out, the proposed functions solve one special case. Better to fix the real problem, which is that the compiler can tell us about this type of error but we're not letting it. That will fix all such problems, not just the pci subsystem.
Just another penny or so.
ron