From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752929AbYLWXTD (ORCPT ); Tue, 23 Dec 2008 18:19:03 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751970AbYLWXSt (ORCPT ); Tue, 23 Dec 2008 18:18:49 -0500 Received: from khc.piap.pl ([195.187.100.11]:57119 "EHLO khc.piap.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751791AbYLWXSs (ORCPT ); Tue, 23 Dec 2008 18:18:48 -0500 To: Linus Torvalds Cc: Harvey Harrison , =?utf-8?B?SMOla29uIEzDuHZkYWw=?= , Hannes Eder , netdev@vger.kernel.org, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement References: <20081222191259.11807.53190.stgit@vmbox.hanneseder.net> <20081222191507.11807.50794.stgit@vmbox.hanneseder.net> <1230053186.1447.9.camel@brick> From: Krzysztof Halasa Date: Wed, 24 Dec 2008 00:18:46 +0100 In-Reply-To: (Linus Torvalds's message of "Tue\, 23 Dec 2008 10\:08\:50 -0800 \(PST\)") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Linus Torvalds writes: > It's more than that. I added the check after some person who had been > programming the kernel (and thus was supposedly fluent in C) literally > could not parse a macro that had "do x while (y)" in it. A literally that simple macro, or a complicated one? I'm all for using brackets when there is / could be some possibility of increasing code readability. E.g. I always use parentheses in a nested if-if, because if (x) if (y) a; else c; may be confusing, especially if formatted differently. if (x) a; else if (y) b; etc. is simple and unambiguous and I don't put the braces. So is a case like do x; while (y); It can't be made more clear with brackets. IOW: improving the style is great. Changing it only to silence some tool is not. > Another example of this is "sizeof". The kernel universally (I hope) has > parenthesis around the sizeof argument, even though it's clearly not > required by the C language. > > It's a coding standard. Right, but they (at least for me) make it more readable. kmalloc(sizeof i) just doesn't look good, the operator looks like a variable name. But there is this return statement. Some people tend to write return (x); I simply write return x; It's clear, and so is a simple do-while. > And quite frankly, anybody who works on gcc has no place complaining about > sparse coding standard warnings. They are a _hell_ of a lot better than > some of the really crazy warnings gcc spews out with "-W". At least the > sparse warnings you can make go away while making the code more > understandable. Some of the -W warnings are unfixable without breaking the > source code. :-) BTW I think I may use sparse differently. I can see false gcc warnings every time the project is being built. OTOH I run sparse only when I have some (almost) completed project (a patch, a driver etc). I make sure the remaining sparse warnings are (from my POV) invalid and it won't spew them on next build again. -- Krzysztof Halasa From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Halasa Date: Tue, 23 Dec 2008 23:18:46 +0000 Subject: Re: [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement Message-Id: List-Id: References: <20081222191259.11807.53190.stgit@vmbox.hanneseder.net> <20081222191507.11807.50794.stgit@vmbox.hanneseder.net> <1230053186.1447.9.camel@brick> In-Reply-To: (Linus Torvalds's message of "Tue\, 23 Dec 2008 10\:08\:50 -0800 \(PST\)") MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Linus Torvalds Cc: Harvey Harrison , =?utf-8?B?SMOla29uIEzDuHZkYWw=?= , Hannes Eder , netdev@vger.kernel.org, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Linus Torvalds writes: > It's more than that. I added the check after some person who had been > programming the kernel (and thus was supposedly fluent in C) literally > could not parse a macro that had "do x while (y)" in it. A literally that simple macro, or a complicated one? I'm all for using brackets when there is / could be some possibility of increasing code readability. E.g. I always use parentheses in a nested if-if, because if (x) if (y) a; else c; may be confusing, especially if formatted differently. if (x) a; else if (y) b; etc. is simple and unambiguous and I don't put the braces. So is a case like do x; while (y); It can't be made more clear with brackets. IOW: improving the style is great. Changing it only to silence some tool is not. > Another example of this is "sizeof". The kernel universally (I hope) has > parenthesis around the sizeof argument, even though it's clearly not > required by the C language. > > It's a coding standard. Right, but they (at least for me) make it more readable. kmalloc(sizeof i) just doesn't look good, the operator looks like a variable name. But there is this return statement. Some people tend to write return (x); I simply write return x; It's clear, and so is a simple do-while. > And quite frankly, anybody who works on gcc has no place complaining about > sparse coding standard warnings. They are a _hell_ of a lot better than > some of the really crazy warnings gcc spews out with "-W". At least the > sparse warnings you can make go away while making the code more > understandable. Some of the -W warnings are unfixable without breaking the > source code. :-) BTW I think I may use sparse differently. I can see false gcc warnings every time the project is being built. OTOH I run sparse only when I have some (almost) completed project (a patch, a driver etc). I make sure the remaining sparse warnings are (from my POV) invalid and it won't spew them on next build again. -- Krzysztof Halasa