Messages in this thread | | | Subject | Re: [PATCH linux-kselftest/test v6] lib/list-test: add a test for the 'list' doubly linked list | From | shuah <> | Date | Fri, 1 Nov 2019 10:49:51 -0600 |
| |
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
| |