Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux Media]     [Older V4L]     [Linux DVB]     [Video Disk Recorder]     [Asterisk]     [Photo]     [DCCP]     [Netdev]     [Xorg]     [Util Linux NG]     [Xfree86]     [Free Photo Albums]     [Fedora Users]     [Fedora Women]     [ALSA Users]     [ALSA Devel]     [SSH]     [Linux USB]

  Powered by Linux