linux-kernel - Re: Merging of completely unreviewed drivers

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: <20080222002945.GF28328@cs181133002.pp.htv.fi>
Date:	Fri, 22 Feb 2008 02:29:45 +0200
From:	Adrian Bunk <bunk@...nel.org>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Krzysztof Halasa <khc@...waw.pl>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Roland Dreier <rdreier@...co.com>,
	Glenn Streiff <gstreiff@...Effect.com>,
	Faisal Latif <flatif@...Effect.com>,
	linux-kernel@...r.kernel.org, general@...ts.openfabrics.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Greg Kroah-Hartman <greg@...ah.com>
Subject: Re: Merging of completely unreviewed drivers

On Thu, Feb 21, 2008 at 11:31:44PM +0000, Alan Cox wrote:
> On Fri, 22 Feb 2008 00:38:14 +0100
> Krzysztof Halasa <khc@...waw.pl> wrote:
> 
> > Linus Torvalds <torvalds@...ux-foundation.org> writes:
> > 
> > > I'm personally of the opinion that a lot of checkpatch "fixes" are 
> > > anything but. That mainly concerns fixing overlong lines
> > 
> > Perhaps we should increase line length limit, 132 should be fine.
> > Especially useful with long printk() lines and long arithmetic
> > expressions.
> 
> Agreed. The fact I'm having to fix bugs introduced by incorrect printk
> wrapping confirms that for printk strings at least it is overzealous.
> 
> I'm all for it complaining about
> 
>   printk(KERN_FOO "<90 chars>", foo, bar + 37);
> 
> type bits when the foo, bar should be underneath to be visible but for
> straight quoted text too long it should not warn and try to get the text
> folded.

I think it should warn, but people have to be aware of the following:
- checkpatch errors are for stuff that really has to be fixed
- checkpatch warnings are for stuff that should be looked at
- the goal is not 0 checkpatch warnings but readable and bugfree code

A nice property of checkpatch is that it encourages to look closer at 
code like the following (it warns about the volatile):

		if (!netif_queue_stopped(netdev)) {
			netif_stop_queue(netdev);
			barrier();
			if ((((((volatile u16)nesnic->sq_tail)+(nesnic->sq_size*2))-nesnic->sq_head) & (nesnic->sq_size - 1)) != 1) {
				netif_start_queue(netdev);
				goto sq_no_longer_full;
			}
		}

> Alan

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--
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/