On Tuesday 02 September 2008 21:37:27 Mike Isely wrote: > On Tue, 2 Sep 2008, Mauro Carvalho Chehab wrote: > > > On Tue, 2 Sep 2008 13:32:17 -0500 (CDT) > > Mike Isely <isely@xxxxxxxxx> wrote: > > > > > I've already argued with Mauro about this multiple times. It's been a > > > waste of time. Really I see two problems here. > > > > > > 1. People (not sure exactly who) are advocating use of a script which > > > does a poor job of enforcing the documented kernel CodingStyle. > > > > No tool is perfect, and for sure there are some situations that it produces > > false results. However, in this case we're discussing, it points to something > > that it seems to be a clear violation on CodingStyle. So, it is not a matter of > > blaming the tool, but discussing the style itself. > > > > It seems that you don't agree that the coding style forbids things like > > if (foo) { > > bar; > > } > > > > If this is the case, you may just argue with checkpatch maintainers. > > > > However, I'm confident that such construction is forbidden at the CodingStyle. > > If I'm right, such discussions need to happen at LKML. > > Mauro: > > You know very well that we've had this discussion before. > > I would much rather do: > > if (foo) bar; > > but checkpatch.pl gets angry about that too. And that IS legal > according to CodingStyle. (Want me to quote chapter and verse? I'll > just find the old e-mail where I already did.) If I am therefore forced > instead to do this: > > if (foo) > bar; > > then it is much safer to do: > > if (foo) { > bar; > } > > instead, but checkpatch.pl hates that too. And it might very well be > wrong in CodingStyle as well - and that is one case where I will be > happy to debate it. But the only reason I even did that was because it > would not let me do: > > if (foo) bar; > > which is really the main problem as far as this issue is concerned. >From the CodingStyle document in Linus' git repo: Don't put multiple statements on a single line unless you have something to hide: if (condition) do_this; do_something_everytime; ... Do not unnecessarily use braces where a single statement will do. if (condition) action(); This does not apply if one branch of a conditional statement is a single statement. Use braces in both branches. if (condition) { do_this(); do_that(); } else { otherwise(); } So the CodingStyle is correctly enforced by checkpatch.pl. > > > > > > 2. Mauro is then using that script as a no-exceptions gatekeeper for all > > > v4l-dvb submissions. > > > > Not really. I do accept patches with broken styles, as I did a > > countless number of times with your patches. > > And every single time you've argued with me about it and every single > time a debate resulted. Is that the process then? I'll just submit > with the bogus (IMHO) warnings and then we'll just have another round of > the same arguments again and again. > > > > > > Yet, I usually complain when this happen too often, as other maintainers used > > to do. > > > > It should be noticed that, from time to time, I heard people from other areas > > in kernel complaining why some Linuxtv patch violates the style (when the > > patch happens to be discussed on another ML). > > > > > The two issues that I repeatedly get tripped on by checkpatch.pl are > > > items which are legal according to CodingStyle. > > > > If you are convinced that this is legal, why don't you just sent an email > > complaining that checkpatch is doing wrong for checking braces on if's? > > I have tried discussing it with others, to no avail. I have seen other > debates on lkml erupt about this. The overall sense I get from it all > is that anyone who argues about checkpatch automatically loses by > default. Apparently one of the rules about checkpatch is that you do > not talk about checkpatch. > > > > > > If there's a bug on checkpatch.pl, it needs to be fixed, instead of tricking > > the source code to avoid it to complain. > > > > > > > I find the use of checkpatch.pl highly > > > offensive because of this, and it has been offending me now for over a > > > year. I'm sick of it, frankly. > > > > I'm also sick of discussing checkpatch.pl. I'm not the maintainer of > > checkpatch.pl or Documentation/CodingStyle. > > But you are inflicting it on others here. So expect to hear something > about it, at least from me. I'm not going to yell at the gun > manufacturer if you're the one pointing the gun at me. It's Mauro's job as subsystem maintainer to enforce such rules. It's like arguing with a cop about a speeding ticket. Utterly pointless. If you want to change the speed limit you argue with the guys who make the law, and that ain't the cop. Mike, I'm sorry to say this but I think this is a senseless discussion. It's a totally unimportant facet of the driver and there are a zillion more important things to worry about. Be glad the kernel doesn't use the GNU codingstyle, that's *really* horrible. Each company or organization has (usually) a coding style, and this is, for better or worse, the Linux coding style. Just close your eyes, remove the curly brackets and commit and get on to the next much more important bug/problem/feature/whatever. Regards, Hans > > > > > > > With regard to the current issue, I will remove the comments and > > > resubmit. > > > > Thanks. This is fine. > > > > > I will not however restructure the code to keep checkpatch.pl > > > happy. > > > > OK. > > > > -Mike > > > > -- > > Mike Isely > isely @ pobox (dot) com > PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 > > _______________________________________________ > v4l-dvb-maintainer mailing list > v4l-dvb-maintainer@xxxxxxxxxxx > http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer > _______________________________________________ v4l-dvb-maintainer mailing list v4l-dvb-maintainer@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer