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

[Marcel Siegert <mws@linuxtv.org>]

Manu Abraham schrieb:
> Marcel Siegert wrote:
> > Manu Abraham schrieb:
> > > Mauro Carvalho Chehab wrote:
> > > > Em Qua, 2007-10-10 Ã s 11:59 -0400, Alan Cox escreveu:
> > > > > On Wed, Oct 10, 2007 at 12:35:41PM -0300, Mauro Carvalho Chehab wrote:
> > > > > > Em Qua, 2007-10-10 Ã s 00:18 -0400, Michael Krufky escreveu:
> > > > > > > Is this illegal as per kernel codingstyle?
> > > > > > Yes, it is. CodingStyle states:
> > > > > <rant>
> > > > > No.. "Illegal" means prohibited by law. Its merely wrong 8)
> > > > > </rant>
> > > > LOL
> > > >
> > > > > > The proper fix is just to replace the offended code by this:
> > > > > >
> > > > > > err=foo();
> > > > > > if (error)
> > > > > >     goto error;
> > > > > Lots of code uses
> > > > >
> > > > >     if ((err = foo()) < 0)
> > > > >
> > > > > so I would'y worry too much. The split one however clearer and also
> > > > > safer.
> > > > Yes, this is not a severe CodingStyle violation. Still, the above code
> > > > is better than the used one.
> > > >
> > > > Since, on your example, it is clear that the programmer wanted to test
> > > > if the value is less than zero.
> > > > The code:
> > > >
> > > >     if ( (err=foo()) )
> > > >
> > > > should also indicate an operator mistake of using =, instead of ==.
> > > >
> > > > Probably, source code analyzers like Coverity will complain about the
> > > > above.
> > > >
> > > > If not violating CodingStyle, I would rather prefer to code this as:
> > > >     if ( !(err=foo() ) or, even better, using:
> > > >     if ( (err=foo()) != 0)
> > > >
> > > > clearly indicating that it is tested if the value is not zero.
> > > >
> > > > Even being a quite simple issue, I would prefer if Jiri can fix it.
> > >
> > > When you have only some few lines of code you can write
> > >
> > >  err = foo()
> > >  if (err) {
> > >   do whatever
> > >  }
> > > doesn't matter ..
> > >
> > > But when you have hell a lot of code, checking error's what you
> > > mention is insane.
> > >
> > > ie,
> > >
> > > if ((err = foo()) expr ) is better.
> > >
> > > http://lkml.org/lkml/2007/2/4/56
> > >
> > > Manu
> > hi manu,
> >
> > it's not worth discussing this in a way like
> > "i know something from the past and that was a different solution".
>
> didn't mean to look at it that way, because i had addressed my concerns 
> at that time as well.
>
> > if you look to other parts in that thread like
> >
> > http://lkml.org/lkml/2007/2/3/150
> >
> > you will see that they came also to a kind of different solution,
> > NOT to use the 1-liner for assignment statements.
> >
> > it's more like a religious/personal discussion how to
> > struct/indent/format code.
> > and, to state my position for clear,
>
>
> It is. Sometimes i find such things in CodingStyle to be too silly.
>
> > if kernel coding style document isnt updated to allow the constructions
> > of code that caused this discussion, we dont have to discuss but follow
> > the rules.
> >
> > anything else on this topic (coding style and it's sense) is to be
> > discussed on kernel ml.
>
> Marcel, It is on LKML.

i do know manu, but as far as i can see from my fresh 2.6.23,
its not solved or changed in vanilla kernel CodingStyle doc.

so we have to follow actual guidelines _or_ wait until
CodingStyle is accordingly updated.

not more, not less.


regards
marcel

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