Re: [v4l-dvb-maintainer] [PATCH 3/3] V4L: cinergyT2, remove bad usage of ERESTARTSYS

[Mauro Carvalho Chehab <mchehab@infradead.org>]

Em Qua, 2007-10-10 às 00:18 -0400, Michael Krufky escreveu:
> 
> Is this illegal as per kernel codingstyle?

Yes, it is. CodingStyle states:

"Don't put multiple statements on a single line unless you have
something to hide"
	and
"Don't put multiple assignments on a single line either.  Kernel coding style
is super simple.  Avoid tricky expressions."

So, Kernel's script/checkpatch.pl is right when complaining about it.

>   I could understand if we may
> want to avoid this sort of thing for the sake of code readability, but
> this seems 100% proper to me, especially considering that we're simply
> trying to catch an error return code.

> One of the things that I really enjoy about the c programming language
> is the fact that you can string many operations together into a single
> statement through the use of logic.

Yes, this is a great C feature, especially to obfuscate a source code.
On C, it is possible to write very complex code, with several
statements, on a single clause, like:

if((c=(a=x,x-=c,++a)>6?1:-1)>0)goto foo;

The above code is valid under C, and won't produce a single compiler
warning. An experienced C programmer will understand the above code,
while non-experienced ones, even with large experience on other
programming languages, may take hours to understand.

A large code, with lots of the above style will be very painful to
analyze, even for advanced programmers. So, especially on big projects,
with lots of contributors, this should really be avoided. 

> I hate the thought of a patch being nacked because of the above.  :-/
> If this is indeed kernel codingstyle, then IMHO, we should let it slide
> for catching error return codes.

It is just a matter of a simple CodingStyle fix.

The proper fix is just to replace the offended code by this:

err=foo();
if (error)
	goto error;

-- 
Cheers,
Mauro