On 10/30/19 1:23 PM, Joe Perches wrote:
On Wed, 2019-10-30 at 22:12 +0300, Dan Carpenter wrote:
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.
checkpatch _is_ working as intended.
It was never intended to be perfect.
checkpatch _always_ depended on a reviewer deciding
whether its output was appropriate.
I don't feel like "checkpatch clean" is a useful criteria for applying
patches.
Nor do I.
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.
That'd be not absolute either because it depended on your
historical judgment as to whether an old warning was in fact
a defect or not.
People make mistakes.
Regex based scripts are by design stupid and untrustworthy.
Mistakes will be made.
Just fix the actual defects in code as soon as possible.
Thanks all for chiming in. I am taking v6 as is and adding an update
to commit log capture the spurious errors from checkpath.pl for this
specific case.
thanks,
-- Shuah
