From: Linus Torvalds <torvalds@linux-foundation.org> To: Krzysztof Halasa <khc@pm.waw.pl> Cc: "Harvey Harrison" <harvey.harrison@gmail.com>, "Håkon Løvdal" <hlovdal@gmail.com>, "Hannes Eder" <hannes@hanneseder.net>, 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 Date: Tue, 23 Dec 2008 15:38:02 -0800 (PST) [thread overview] Message-ID: <alpine.LFD.2.00.0812231529030.3535@localhost.localdomain> (raw) In-Reply-To: <m3hc4usco9.fsf@maximus.localdomain> On Wed, 24 Dec 2008, Krzysztof Halasa wrote: > > So is a case like > do > x; > while (y); > It can't be made more clear with brackets. Oh yes it can. People look at that, and it's so uncommon that they literally believe it is a mis-indent. Your example with nested if-statements are totally pointless, because you didn't even apparently understand my comment about "while()" having two totally different meanings (which is not true of "if()"), nor realize the importance of how common something is. Common patterns become things that people take for granted and don't have any trouble with. In contrast, do-while without braces is _extremely_ uncommon. > IOW: improving the style is great. Changing it only to silence some > tool is not. Sorry, you're wrong. It's not changed to silence some tool. THIS IS THE KERNEL CODING STYLE. I don't care one whit about your personal coding style. The kernel has brances. End of discussion. sparse complains about lack of them. Comprende? > Right, but they (at least for me) make it more readable. I don't care. > kmalloc(sizeof i) just doesn't look good, the operator looks like > a variable name. And I agree with you. "sizeof i" doesn't look good. It's uncommon, and doesn't match peoples expectations. > But there is this return statement. Some people tend to write > return (x); I simply write return x; I do to. And it's the common thing to do, and only totally confused people think that 'return' is a somehow remotely like a "function" of its arguments (the way 'sizeof' is - sizeof _is_ a function of its arguments, albeit a very rare one). > It's clear, and so is a simple do-while. No. "return x;" is clear because it's the common thing, which means that peopel are good at reading it. Another example of "common vs non-common" is this: if (0 <= x) do something.. is something that crazy people do (sadly, one of the crazy people taught the git maintainer C programming, so now even sane people do it). It's crazy because it's uncommon, which means that most people have to think about it A LOT MORE than about if (x >= 0) do something.. even though technically both are obviously EXACTLY THE SAME THING. Can you see the argument? Doing things the common way is important, because it allows people to see what they mean without having to think about it. They just scan it, and the meaning is clear. And that's why "do while" without braces is bad. If you scan it quickly on its own, you may well end up just seeing the while (x); part, and get confused ("oh, a delay loop"). But if you see } while (x); you aren't confused, because the latter one is clearly an ending condition of a do-while loop, IN A WAY THAT THE FIRST ONE IS NOT! See? do-while is very special, because as mentioned, "while" is a really magic C keyword that has two TOTALLY DIFFERENT meanings. Don't make people look for the "do". Linus
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org> To: Krzysztof Halasa <khc@pm.waw.pl> Cc: "Harvey Harrison" <harvey.harrison@gmail.com>, "Håkon Løvdal" <hlovdal@gmail.com>, "Hannes Eder" <hannes@hanneseder.net>, 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 Date: Tue, 23 Dec 2008 23:38:02 +0000 [thread overview] Message-ID: <alpine.LFD.2.00.0812231529030.3535@localhost.localdomain> (raw) In-Reply-To: <m3hc4usco9.fsf@maximus.localdomain> On Wed, 24 Dec 2008, Krzysztof Halasa wrote: > > So is a case like > do > x; > while (y); > It can't be made more clear with brackets. Oh yes it can. People look at that, and it's so uncommon that they literally believe it is a mis-indent. Your example with nested if-statements are totally pointless, because you didn't even apparently understand my comment about "while()" having two totally different meanings (which is not true of "if()"), nor realize the importance of how common something is. Common patterns become things that people take for granted and don't have any trouble with. In contrast, do-while without braces is _extremely_ uncommon. > IOW: improving the style is great. Changing it only to silence some > tool is not. Sorry, you're wrong. It's not changed to silence some tool. THIS IS THE KERNEL CODING STYLE. I don't care one whit about your personal coding style. The kernel has brances. End of discussion. sparse complains about lack of them. Comprende? > Right, but they (at least for me) make it more readable. I don't care. > kmalloc(sizeof i) just doesn't look good, the operator looks like > a variable name. And I agree with you. "sizeof i" doesn't look good. It's uncommon, and doesn't match peoples expectations. > But there is this return statement. Some people tend to write > return (x); I simply write return x; I do to. And it's the common thing to do, and only totally confused people think that 'return' is a somehow remotely like a "function" of its arguments (the way 'sizeof' is - sizeof _is_ a function of its arguments, albeit a very rare one). > It's clear, and so is a simple do-while. No. "return x;" is clear because it's the common thing, which means that peopel are good at reading it. Another example of "common vs non-common" is this: if (0 <= x) do something.. is something that crazy people do (sadly, one of the crazy people taught the git maintainer C programming, so now even sane people do it). It's crazy because it's uncommon, which means that most people have to think about it A LOT MORE than about if (x >= 0) do something.. even though technically both are obviously EXACTLY THE SAME THING. Can you see the argument? Doing things the common way is important, because it allows people to see what they mean without having to think about it. They just scan it, and the meaning is clear. And that's why "do while" without braces is bad. If you scan it quickly on its own, you may well end up just seeing the while (x); part, and get confused ("oh, a delay loop"). But if you see } while (x); you aren't confused, because the latter one is clearly an ending condition of a do-while loop, IN A WAY THAT THE FIRST ONE IS NOT! See? do-while is very special, because as mentioned, "while" is a really magic C keyword that has two TOTALLY DIFFERENT meanings. Don't make people look for the "do". Linus
next prev parent reply other threads:[~2008-12-23 23:38 UTC|newest] Thread overview: 149+ messages / expand[flat|nested] mbox.gz Atom feed top 2008-12-22 19:14 [PATCH 00/27] drivers/net: fix sparse warnings Hannes Eder 2008-12-22 19:14 ` Hannes Eder 2008-12-22 19:14 ` [PATCH 01/27] drivers/net: fix sparse warning: use ANSI-style function declaration Hannes Eder 2008-12-22 19:14 ` [PATCH 01/27] drivers/net: fix sparse warning: use ANSI-style Hannes Eder 2008-12-26 7:53 ` [PATCH 01/27] drivers/net: fix sparse warning: use ANSI-style function declaration David Miller 2008-12-26 7:53 ` [PATCH 01/27] drivers/net: fix sparse warning: use ANSI-style David Miller 2008-12-22 19:15 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement Hannes Eder 2008-12-22 19:15 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a Hannes Eder 2008-12-22 22:14 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement Krzysztof Halasa 2008-12-22 22:14 ` Krzysztof Halasa 2008-12-22 23:44 ` Håkon Løvdal 2008-12-22 23:44 ` Håkon Løvdal 2008-12-22 23:44 ` Håkon Løvdal 2008-12-23 16:31 ` Krzysztof Halasa 2008-12-23 16:31 ` Krzysztof Halasa 2008-12-23 17:26 ` Harvey Harrison 2008-12-23 17:26 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while Harvey Harrison 2008-12-23 17:36 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement Krzysztof Halasa 2008-12-23 17:36 ` Krzysztof Halasa 2008-12-23 18:08 ` Linus Torvalds 2008-12-23 18:08 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while Linus Torvalds 2008-12-23 23:18 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement Krzysztof Halasa 2008-12-23 23:18 ` Krzysztof Halasa 2008-12-23 23:38 ` Linus Torvalds [this message] 2008-12-23 23:38 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while Linus Torvalds 2008-12-24 2:03 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement Krzysztof Halasa 2008-12-24 2:03 ` Krzysztof Halasa 2008-12-24 2:10 ` Linus Torvalds 2008-12-24 2:10 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while Linus Torvalds 2008-12-25 17:02 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement Krzysztof Halasa 2008-12-25 17:02 ` Krzysztof Halasa 2008-12-25 6:17 ` Junio C Hamano 2008-12-25 6:17 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a Junio C Hamano 2008-12-29 14:35 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement Hannes Eder 2008-12-29 14:35 ` Hannes Eder 2008-12-26 7:55 ` David Miller 2008-12-26 7:55 ` [PATCH 02/27] drivers/net: fix sparse warnings: make do-while David Miller 2008-12-22 19:15 ` [PATCH 03/27] drivers/net: fix sparse warning: returning void-valued expression Hannes Eder 2008-12-22 19:15 ` [PATCH 03/27] drivers/net: fix sparse warning: returning void-valued Hannes Eder 2008-12-26 0:17 ` [PATCH 03/27] drivers/net: fix sparse warning: returning void-valued expression David Miller 2008-12-26 0:17 ` [PATCH 03/27] drivers/net: fix sparse warning: returning David Miller 2008-12-26 14:39 ` [PATCH 03/27] drivers/net: fix sparse warning: returning void-valued expression Hannes Eder 2008-12-26 14:39 ` Hannes Eder 2008-12-26 19:59 ` Randy Dunlap 2008-12-26 19:59 ` [PATCH 03/27] drivers/net: fix sparse warning: returning Randy Dunlap 2008-12-27 19:11 ` [PATCH 03/27] drivers/net: fix sparse warning: returning void-valued expression Hannes Eder 2008-12-27 19:11 ` Hannes Eder 2008-12-27 19:20 ` Sam Ravnborg 2008-12-27 19:20 ` Sam Ravnborg 2008-12-27 21:38 ` [PATCH] Makefile: disable sparse warning "returning void-valued expression" Hannes Eder 2008-12-27 21:38 ` Hannes Eder 2008-12-27 21:57 ` Sam Ravnborg 2008-12-27 21:57 ` Sam Ravnborg 2008-12-26 7:56 ` [PATCH 03/27] drivers/net: fix sparse warning: returning void-valued expression David Miller 2008-12-26 7:56 ` [PATCH 03/27] drivers/net: fix sparse warning: returning David Miller 2008-12-22 19:15 ` [PATCH 04/27] drivers/net: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:15 ` Hannes Eder 2008-12-26 7:57 ` David Miller 2008-12-26 7:57 ` [PATCH 04/27] drivers/net: fix sparse warnings: make symbols David Miller 2008-12-22 19:15 ` [PATCH 05/27] drivers/net/arcnet: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:15 ` [PATCH 05/27] drivers/net/arcnet: fix sparse warnings: make symbols Hannes Eder 2008-12-26 7:57 ` [PATCH 05/27] drivers/net/arcnet: fix sparse warnings: make symbols static David Miller 2008-12-26 7:57 ` [PATCH 05/27] drivers/net/arcnet: fix sparse warnings: make David Miller 2008-12-22 19:15 ` [PATCH 06/27] drivers/net/atlx: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:15 ` [PATCH 06/27] drivers/net/atlx: fix sparse warnings: make symbols Hannes Eder 2008-12-26 7:58 ` [PATCH 06/27] drivers/net/atlx: fix sparse warnings: make symbols static David Miller 2008-12-26 7:58 ` [PATCH 06/27] drivers/net/atlx: fix sparse warnings: make David Miller 2008-12-22 19:15 ` [PATCH 07/27] drivers/net/bonding: fix sparse warnings: move decls to header file Hannes Eder 2008-12-22 19:15 ` [PATCH 07/27] drivers/net/bonding: fix sparse warnings: move decls to Hannes Eder 2008-12-26 7:59 ` [PATCH 07/27] drivers/net/bonding: fix sparse warnings: move decls to header file David Miller 2008-12-26 7:59 ` [PATCH 07/27] drivers/net/bonding: fix sparse warnings: move David Miller 2008-12-22 19:16 ` [PATCH 08/27] drivers/net/cxgb3: comment out dead code Hannes Eder 2008-12-22 19:16 ` Hannes Eder 2008-12-26 7:59 ` David Miller 2008-12-26 7:59 ` David Miller 2008-12-22 19:16 ` [PATCH 09/27] drivers/net/e1000e: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:16 ` [PATCH 09/27] drivers/net/e1000e: fix sparse warnings: make symbols Hannes Eder 2008-12-22 19:16 ` [PATCH 10/27] drivers/net/enic: fix sparse warning: make symbol static Hannes Eder 2008-12-22 19:16 ` Hannes Eder 2008-12-26 8:01 ` David Miller 2008-12-26 8:01 ` [PATCH 10/27] drivers/net/enic: fix sparse warning: make David Miller 2008-12-22 19:16 ` [PATCH 11/27] drivers/net/igb: remove dead code (function 'igb_read_pci_cfg') Hannes Eder 2008-12-22 19:16 ` [PATCH 11/27] drivers/net/igb: remove dead code (function Hannes Eder 2008-12-22 22:30 ` [PATCH 11/27] drivers/net/igb: remove dead code (function 'igb_read_pci_cfg') Jeff Kirsher 2008-12-22 22:30 ` Jeff Kirsher 2008-12-26 8:03 ` David Miller 2008-12-26 8:03 ` [PATCH 11/27] drivers/net/igb: remove dead code (function David Miller 2008-12-22 19:16 ` [PATCH 12/27] drivers/net/irda: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:16 ` [PATCH 12/27] drivers/net/irda: fix sparse warnings: make symbols Hannes Eder 2008-12-26 8:03 ` [PATCH 12/27] drivers/net/irda: fix sparse warnings: make symbols static David Miller 2008-12-26 8:03 ` [PATCH 12/27] drivers/net/irda: fix sparse warnings: make David Miller 2008-12-22 19:16 ` [PATCH 13/27] drivers/net/ixgbe: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:16 ` [PATCH 13/27] drivers/net/ixgbe: fix sparse warnings: make symbols Hannes Eder 2008-12-26 8:04 ` [PATCH 13/27] drivers/net/ixgbe: fix sparse warnings: make symbols static David Miller 2008-12-26 8:04 ` [PATCH 13/27] drivers/net/ixgbe: fix sparse warnings: make David Miller 2008-12-22 19:16 ` [PATCH 14/27] drivers/net/netxen: fix sparse warnings: use NULL pointer instead of plain integer Hannes Eder 2008-12-22 19:16 ` [PATCH 14/27] drivers/net/netxen: fix sparse warnings: use NULL Hannes Eder 2008-12-26 8:04 ` [PATCH 14/27] drivers/net/netxen: fix sparse warnings: use NULL pointer instead of plain integer David Miller 2008-12-26 8:04 ` [PATCH 14/27] drivers/net/netxen: fix sparse warnings: use David Miller 2008-12-22 19:17 ` [PATCH 15/27] drivers/net/qlge: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:17 ` [PATCH 15/27] drivers/net/qlge: fix sparse warnings: make symbols Hannes Eder 2008-12-26 8:05 ` [PATCH 15/27] drivers/net/qlge: fix sparse warnings: make symbols static David Miller 2008-12-26 8:05 ` [PATCH 15/27] drivers/net/qlge: fix sparse warnings: make David Miller 2008-12-22 19:17 ` [PATCH 16/27] drivers/net/skfp: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:17 ` [PATCH 16/27] drivers/net/skfp: fix sparse warnings: make symbols Hannes Eder 2008-12-26 8:06 ` [PATCH 16/27] drivers/net/skfp: fix sparse warnings: make symbols static David Miller 2008-12-26 8:06 ` [PATCH 16/27] drivers/net/skfp: fix sparse warnings: make David Miller 2008-12-22 19:17 ` [PATCH 17/27] drivers/net/tokenring: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:17 ` [PATCH 17/27] drivers/net/tokenring: fix sparse warnings: make Hannes Eder 2008-12-26 8:07 ` [PATCH 17/27] drivers/net/tokenring: fix sparse warnings: make symbols static David Miller 2008-12-26 8:07 ` [PATCH 17/27] drivers/net/tokenring: fix sparse warnings: make David Miller 2008-12-22 19:17 ` [PATCH 18/27] drivers/net/tulip: fix sparse warnings: make do-while a compound statement Hannes Eder 2008-12-22 19:17 ` [PATCH 18/27] drivers/net/tulip: fix sparse warnings: make do-while a Hannes Eder 2008-12-26 8:07 ` [PATCH 18/27] drivers/net/tulip: fix sparse warnings: make do-while a compound statement David Miller 2008-12-26 8:07 ` [PATCH 18/27] drivers/net/tulip: fix sparse warnings: make David Miller 2008-12-22 19:17 ` [PATCH 19/27] drivers/net/usb: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:17 ` [PATCH 19/27] drivers/net/usb: fix sparse warnings: make symbols Hannes Eder 2008-12-26 8:08 ` [PATCH 19/27] drivers/net/usb: fix sparse warnings: make symbols static David Miller 2008-12-26 8:08 ` [PATCH 19/27] drivers/net/usb: fix sparse warnings: make David Miller 2008-12-22 19:17 ` [PATCH 20/27] drivers/net/wan: fix sparse warnings: make do-while a compound statement Hannes Eder 2008-12-22 19:17 ` [PATCH 20/27] drivers/net/wan: fix sparse warnings: make do-while a Hannes Eder 2008-12-22 22:09 ` [PATCH 20/27] drivers/net/wan: fix sparse warnings: make do-while a compound statement Krzysztof Halasa 2008-12-22 22:09 ` Krzysztof Halasa 2008-12-22 19:18 ` [PATCH 21/27] drivers/net/wan: fix sparse warning: make symbol static Hannes Eder 2008-12-22 19:18 ` Hannes Eder 2008-12-26 8:11 ` David Miller 2008-12-26 8:11 ` [PATCH 21/27] drivers/net/wan: fix sparse warning: make symbol David Miller 2008-12-22 19:18 ` [PATCH 22/27] drivers/net/wan/z85230.c: fix sparse warnings: un-EXPORT symbols Hannes Eder 2008-12-22 19:18 ` [PATCH 22/27] drivers/net/wan/z85230.c: fix sparse warnings: Hannes Eder 2008-12-26 8:12 ` [PATCH 22/27] drivers/net/wan/z85230.c: fix sparse warnings: un-EXPORT symbols David Miller 2008-12-26 8:12 ` [PATCH 22/27] drivers/net/wan/z85230.c: fix sparse warnings: David Miller 2008-12-22 19:18 ` [PATCH 23/27] drivers/net/wireless: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:18 ` [PATCH 23/27] drivers/net/wireless: fix sparse warnings: make symbols Hannes Eder 2008-12-26 8:13 ` [PATCH 23/27] drivers/net/wireless: fix sparse warnings: make symbols static David Miller 2008-12-26 8:13 ` [PATCH 23/27] drivers/net/wireless: fix sparse warnings: make David Miller 2008-12-22 19:18 ` [PATCH 24/27] drivers/net/wireless/ath9k: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:18 ` [PATCH 24/27] drivers/net/wireless/ath9k: fix sparse warnings: make Hannes Eder 2008-12-22 19:18 ` [PATCH 25/27] drivers/net/wireless/b43: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:18 ` [PATCH 25/27] drivers/net/wireless/b43: fix sparse warnings: make Hannes Eder 2008-12-26 8:13 ` [PATCH 25/27] drivers/net/wireless/b43: fix sparse warnings: make symbols static David Miller 2008-12-26 8:13 ` [PATCH 25/27] drivers/net/wireless/b43: fix sparse warnings: David Miller 2008-12-22 19:18 ` [PATCH 26/27] drivers/net/wireless/ipw2x00: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:18 ` [PATCH 26/27] drivers/net/wireless/ipw2x00: fix sparse warnings: make Hannes Eder 2008-12-26 8:14 ` [PATCH 26/27] drivers/net/wireless/ipw2x00: fix sparse warnings: make symbols static David Miller 2008-12-26 8:14 ` [PATCH 26/27] drivers/net/wireless/ipw2x00: fix sparse David Miller 2008-12-22 19:19 ` [PATCH 27/27] drivers/net/wireless/prism54: fix sparse warnings: make symbols static Hannes Eder 2008-12-22 19:19 ` [PATCH 27/27] drivers/net/wireless/prism54: fix sparse warnings: make Hannes Eder 2008-12-26 8:15 ` [PATCH 27/27] drivers/net/wireless/prism54: fix sparse warnings: make symbols static David Miller 2008-12-26 8:15 ` [PATCH 27/27] drivers/net/wireless/prism54: fix sparse David Miller
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.LFD.2.00.0812231529030.3535@localhost.localdomain \ --to=torvalds@linux-foundation.org \ --cc=hannes@hanneseder.net \ --cc=harvey.harrison@gmail.com \ --cc=hlovdal@gmail.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=khc@pm.waw.pl \ --cc=linux-kernel@vger.kernel.org \ --cc=netdev@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.