netdev - Re: [net-next-2.6 04/15] e1000e: checkpatch error

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 for Android: free password hash cracker in your pocket

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

Date:	Sat, 25 Dec 2010 14:10:38 +0100
From:	Krzysztof Halasa <khc@...waw.pl>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	jeffrey.t.kirsher@...el.com, davem@...emloft.net,
	Bruce Allan <bruce.w.allan@...el.com>, netdev@...r.kernel.org,
	gospo@...hat.com, bphilips@...ell.com
Subject: Re: [net-next-2.6 04/15] e1000e: checkpatch error - open braces

Jeff Garzik <jeff@...zik.org> writes:

>> ERROR: that open brace { should be on the previous line

>> -static const u16 e1000_gg82563_cable_length_table[] =
>> -	 { 0, 60, 115, 150, 150, 60, 115, 150, 180, 180, 0xFF };

>> -	static const u32 test[] =
>> -		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};

> Gah, that looks worse than the unpatched code.
>
> I think this was an over-literal checkpatch interpretation.  If you
> look at e.g. drivers/net/tg3.c, you see that longer tables have the
> trailing braces on a separate line, where they should be.
>
> IMO, the single-line cases should just be left as they are now in the
> source code.

I think so. Also I think it's a false positive - the above aren't the
typical
	if()
		{
		xxx
		}

They are instead single lines:
	type var[] = {a, b, c, d};
broken into two to meet the (former) line length limit.

Makes me think that we should never break long lines in the code. This
is best left for the text editors and viewers (only while displaying).
-- 
Krzysztof Halasa
--
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