On Thu, 12 Jul 2007, Andy Whitcroft wrote: > > printk() should include KERN_ facility level > > #750: FILE: drivers/serial/sb1250-duart.c:675: > > + printk(err); > > Heh, yeah Ingo pointed this style out. This is a wrapper where the > facility will be supplied by the caller (I assume). The thought there Actually "err" is "static const char *", except it is used twice in the function, so rather than cluttering the source with two identical strings and relying on GCC merging them I did it explicitly. > was that only complain on printks which had a string literal as their > first arguement. That gets us very high accuracy and eliminates these > falsies. That would be my suggestion too, if you asked me. But as did not, I do not either. > I think I tend to agree that the MAKEMASK ones are separate. Good to > see someone using their common sense in the face of whinging by the tool. Thanks for appreciation. ;-) > WARNING: declaring multiple variables together should be avoided > #372: FILE: drivers/serial/sb1250-duart.c:246: > + unsigned int mctrl, status; Well, this is probably superfluous -- why would anyone prefer: int r0; int r1; int r2; int r3; int r4; to: int r0, r1, r2, r3, r4; unconditionally? I agree clustering variable declarations may obfuscate the code, but then again, a bit of common sense should be used. It usually makes sense to group related variables together and declare other ones separately. And obviously if somebody writes unreadable code, then it is hard to change the habit with a script no matter how much you try. Maciej
