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.>