linux-kernel - Re: [PATCH] update checkpatch.pl to version 0.03

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 05 Jun 2007 19:45:20 +0100
From:	Andy Whitcroft <apw@...dowen.org>
To:	jschopp <jschopp@...tin.ibm.com>
CC:	Andrew Morton <akpm@...l.org>, Randy Dunlap <rdunlap@...otime.net>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] update checkpatch.pl to version 0.03

jschopp wrote:
>> The original suggestion was to count them and only complain if there
>> were "lots".  I had thought though that the general consensus was that
>> #ifdef in C files was pretty much frowned upon.  I must admit to working
>> to the "you must be able to justify all winges in the output" rather
>> than expecting the result to be empty necessarily.
> 
> I really think we should work towards, "if you see an error the odds are
> overwhelming that we aren't wasting your time and you should fix this."
> rather than, "every time you send a patch you will get a couple false
> positives that you will have to think about and justify to yourself,
> slowing down your sending the patch out and making more work for you". 
> I'm not saying we have to have 100% accuracy, but I want it well in the
> 90s.

Yeah I tend to agree and most of my work to date has been squashing
false positives.

> Now back to the ifdef's.  I don't think we can really even say a lot or
> a little is broken (short patch can do 1 ifdef that is stupid, long
> patch could do several that are good).  I think we'll either have to let
> that one go or put it under a non-default flag.    We do still have
> humans reviewing code who can make judgement calls like if you #ifdefs
> are good or not.
> 
> What we can check for is #if 0 code.  I hate that one.

For now I've disabled this one.  Put it in the freezer with the
StudlyCaps one.

I like the idea of checking for #if 0 as that is very likely bogus in a
patch meant for inclusion.

>> We've not talked about how the tool might grow.  My thought was that
>> soon we would enable the robot replies on linux-mm (say) and use the
>> feedback from that to tune what we do and do not report.  I have been
>> pushing all of the contributions to -mm for sometime through it and
>> cirtainly the #ifdef one once of the more common ones (other than white
>> space dammage and >80 character lines).
> 
> If you have reasonably good SPAM filters on your list and make the robot
> so it is very good about only picking up mail that really are patches
> then this could be a very good idea. Just be careful.  I could send out
> a bunch of mail with fake headers saying it was from say Andy Whitcroft,
> then the robot replies and you got a bunch of junk mail.

All very good points.  It does only reply to emails which are clearly
unidiff patches and silently drops all else.  But the potential for spam
is worrysome ... will think on this some and see if we can come up with
a safety net.

> User feedback is really useful, both for us and for the user.  Based on
> user feedback from the very small number of users I had I tweaked a lot
> of regular expressions, added some new checks, and removed some I could
> never get right.

I am getting a fair bit of feedback, most of it copied to Joel and Randy
so at least that bit is working.

-apw
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/