netdev - Re: Treewide frequency of various checkpatch messages

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]

Message-ID: <20140310193346.GA2850@kroah.com>
Date:	Mon, 10 Mar 2014 12:33:46 -0700
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Joe Perches <joe@...ches.com>
Cc:	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	apw@...onical.com, devel@...uxdriverproject.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Miller <davem@...emloft.net>,
	Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: Treewide frequency of various checkpatch messages

On Mon, Mar 10, 2014 at 11:48:50AM -0700, Joe Perches wrote:
> On Mon, 2014-03-10 at 09:50 -0700, Greg KH wrote:
> > On Mon, Mar 10, 2014 at 09:02:26AM -0700, Joe Perches wrote:
> > > On Fri, 2014-03-07 at 01:30 -0800, Joe Perches wrote:
> > > > On Fri, 2014-03-07 at 10:54 +0300, Dan Carpenter wrote:
> > >  (a question about a new message warning of a missing
> > >   blank line between variable declaration blocks and
> > >   code in a function)
> > > > > How many warnings does this generate does this generate when you run it
> > > > > across the whole tree?
> > > > A lot.
> > > 
> > > Turns out it's 20,210 and it's the 14th
> > > most common checkpatch message type.
> > > 
> > >     14    20210  WARNING:SPACING: Missing a blank line after declarations
> > 
> > I think it's still worthwhile to clean up.
> 
> Maybe.
> 
> Luckily, <smile> I don't have to deal with the
> patches that would be generated by this message.
> 
> Some people are going to view patches for this as
> useless noise.

That's true for all checkpatch cleanups :)

> Couple of things:
> 
> It's kind of interesting how the messages vary by
> subsystem.  Let me know if you want any breakdowns.
> 
> And there are a small number of false positives for
> this "Missing a blank line" test with declarations
> like:
> 
> 	typedef *foo;
> 	DECLARE_BITMAP(foo);
> 	__DECL_REG(foo);
> 	LIST_HEAD(foo);
> 
> So there could be a minor improvement to the test.
> 
> I looked at some of the results using:
> 
> This sort of match stands out a bit:
> 
> ---> arch/tile/lib/spinlock_32.c:68:
> {
>         u32 iterations = 0;
>         while (arch_spin_is_locked(lock))
>                 delay_backoff(iterations++);
> }
> 
> Instances like this may be fine, but adding blank
> lines to very short functions with a single
> declaration just adds to the overall line count.
> 
> I've no strong opinion of the need to write code
> like:
> 
> {
>         u32 iterations = 0;
> 
>         while (arch_spin_is_locked(lock))
>                 delay_backoff(iterations++);
> }

I wonder if there's a way to "count" the size of the function, and only
complain if it's longer than 4-5 lines long?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html