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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 10 Oct 2007 12:35:41 -0300
From:	Mauro Carvalho Chehab <mchehab@...radead.org>
To:	Michael Krufky <mkrufky@...uxtv.org>
Cc:	video4linux-list@...hat.com, Jiri Slaby <jirislaby@...il.com>,
	daniel@...u.de, linux-kernel@...r.kernel.org, holger@...u.de,
	v4l-dvb maintainer list <v4l-dvb-maintainer@...uxtv.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [v4l-dvb-maintainer] [PATCH 3/3] V4L: cinergyT2, remove bad
	usage of ERESTARTSYS

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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/