On Wed, Oct 30, 2019 at 10:27:12AM -0600, shuah wrote:
It's better to ignore checkpatch and other scripts when they are wrong. (unless the warning message inspires you to make the code more readable for humans).
It gets confusing when to ignore and when not to. It takes work to figure out and it is subjective.
In this case, it's not subjective because checkpatch is clearly not working as intended.
I don't feel like "checkpatch clean" is a useful criteria for applying patches. If someone sends a patch and I can spot a bunch of checkpatch issues with my bare eyeballs then I get slightly annoyed for wasting my time. But as a reviewer, I mostly care about my own judgement. Can I understand what the code is doing? It is subjective, but I'm smarter than a Perl script and I try to be kind to people.
The other things about warnings is that I always encourage people to just ignore old warnings. If you're running Smatch and you see a warning in ancient code that means I saw it five years ago and didn't fix it so it's a false positive. Old warnings are always 100% false positives.
regards, dan carpenter