Messages in this thread | | | Date | Wed, 10 Oct 2007 20:04:16 +0200 | From | Marcel Siegert <> | Subject | Re: [v4l-dvb-maintainer] [PATCH 3/3] V4L: cinergyT2, remove bad usage of ERESTARTSYS |
| |
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/
| |