On Wed, May 28, 2014 at 12:12 PM, Joel Sherrill <joel.sherr...@oarcorp.com> wrote: > On 5/28/2014 10:44 AM, Gedare Bloom wrote: >> >> On Wed, May 28, 2014 at 11:43 AM, Gedare Bloom <ged...@rtems.org> wrote: >>> >>> On Wed, May 28, 2014 at 11:32 AM, Sebastian Huber >>> <sebastian.hu...@embedded-brains.de> wrote: >>>> >>>> On 2014-05-20 20:43, Sebastian Huber wrote: >>>>> >>>>> On 05/20/2014 05:30 PM, Joel Sherrill wrote: >>>>>> >>>>>> On 5/20/2014 10:17 AM, Sebastian Huber wrote: >>>>>>>> >>>>>>>> On 2014-05-20 16:58, Joel Sherrill wrote: >>>>>>>> >>>>>>>>>> On 5/13/2014 1:16 AM, Sebastian Huber wrote: >>>>>>>>>> >>>>>>>>>>>>>> On 2014-05-13 01:28, Joel Sherrill wrote: >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Hi >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Both schedulerident.c and schedulergetprocessorset.c do >>>>>>>>>>>>>>>>>> not >>>>>>>>>>>>>>>>>> follow >>>>>>>>>>>>>>>>>> RTEMS Coding Conventions on avoidance of deep nesting by >>>>>>>>>>>>>>>>>> using >>>>>>>>>>>>>>>>>> early returns. >>>>>>>>>>>>>> >>>>>>>>>>>>>> I don't find such a rule here: >>>>>>>>>>>>>> >>>>>>>>>>>>>> http://www.rtems.org/wiki/index.php/Coding_Conventions >>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> The nesting on schedulergetprocessorset.c is pretty >>>>>>>>>>>>>>>>>> ugly and could have been avoided easily. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>> I prefer a single exit point. This is also advised by MISRA >>>>>>>>>>>>>> Rule >>>>>>>>>> >>>>>>>>>> 15.5. It is >>>>>>>>>>>>>> >>>>>>>>>>>>>> required by ISO 61508 and ISO 26262. It is also "will" rule >>>>>>>>>>>>>> (AV >>>>>>>>>>>>>> Rule >>>>>>>>>> >>>>>>>>>> 113) of >>>>>>>>>>>>>> >>>>>>>>>>>>>> the "JOINT STRIKE FIGHTER AIR VEHICLE C++ CODING STANDARDS FOR >>>>>>>>>>>>>> THE >>>>>>>>>> >>>>>>>>>> SYSTEM >>>>>>>>>>>>>> >>>>>>>>>>>>>> DEVELOPMENT AND DEMONSTRATION PROGRAM". >>>>>>>>>>>>>> >>>>>>>>>>>>>> http://www.stroustrup.com/JSF-AV-rules.pdf >>>>>>>>>>>>>> >>>>>>>>>>>>>> So if you ask me, then we should also add this to the our >>>>>>>>>>>>>> coding >>>>>>>>>> >>>>>>>>>> conventions. >>>>>>>>>> I'll be honest. I don't really care what the JSF rules or MISRA >>>>>>>>>> rules >>>>>>>>>> say. At this point in the "discussion", what they say is >>>>>>>>>> completely >>>>>>>>>> irrelevant to this situation. Whether I like this rule or not, the >>>>>>>>>> fact >>>>>>>>>> remains that >>>>>>>>>> >>>>>>>>>> (1) There was no community discussion. >>>>>>>> >>>>>>>> I made several attempts to improve the coding style on the wiki. >>>>>>>> This >>>>>>>> is an >>>>>>>> ongoing process. >>>>>>>> >>>>>>>> >>>>>>>>>> (2) There is no way to enforce it (or any other rule) >>>>>>>>>> (3) It is an arbitrary change and leads to inconsistent >>>>>>>>>> code in the code base. >>>>>>>>>> >>>>>>>>>> (1) and (3) really bother me. (2) we have lived with a long time. >>>>>>>>>> I don't like it but we haven't figured out how to solve that. >>>>>>>>>> >>>>>>>>>> The only community discussion was me asking why you didn't >>>>>>>>>> follow existing practice. >>>>>>>> >>>>>>>> There are so many coding styles in the RTEMS code base that it is >>>>>>>> hard >>>>>>>> to >>>>>>> >>>>>>> guess >>>>>>>> >>>>>>>> the "existing practice". >>>>>> >>>>>> The BSPs and drivers are not good examples of anything and we have all >>>>>> long stated that. The APIs and score portion of cpukit was reasonably >>>>>> consistent. >>>>>> >>>>>> Your response is simply that "I couldn't find the answer so I did what >>>>>> I >>>>>> wanted" >>>>> >>>>> >>>>> You said I violate a rule and I said that there is no such rule in the >>>>> coding >>>>> style >>>>> >>>>> http://www.rtems.org/wiki/index.php/Coding_Conventions >>>>> >>>>> and that there are good reasons to not follow such a rule. It was >>>>> simply >>>>> not >>>>> clear to me that were was a "avoidance of deep nesting by using early >>>>> returns" >>>>> rule. If I look now at the code, then there are indeed a lot of >>>>> examples >>>>> for >>>>> this rule. >>>>> >>>>> All I ask is that rules that should be enforced should be part of the >>>>> coding >>>>> style. This lack of a proper coding style makes it hard for newcomers >>>>> (e.g. >>>>> GSoC students or new colleagues) to get started with RTEMS development. >>>>> I >>>>> leads also to discussions like this. >>>> >>>> >>>> We still have no new rule for "avoidance of deep nesting by using early >>>> returns" in the wiki page. So we are still in the situation that >>>> everyone >>>> interested in code contributions must deduce this rule himself from the >>>> "almost 200 API calls which take object ID and have the _Object_Get() >>>> switch". >>>> >>> My vote is to limit nesting to 4 levels of indentation. Early returns >>> or local goto to error/out label should be used when it makes sense. >>> For example: >>> int foo(int bar) >>> { >>> int a = 0; // one level >>> if ( bar == a ) { >>> int b = 1; // two levels >>> while ( a++ < b ) { >>> int c = 2; // three levels >>> if ( a == c ) { >>> goto err; // fourth and deepest level! >>> } >>> } >>> } >>> return 0; >>> err: >>> return -1; >>> } >>> >> Ahem, the goto should be used primarily when there is cleanup to be >> done. In this example it would be better to return the -1 directly. >> Still, this is my vote. > > Agreed on the use of gotos. Avoid. > > But whether N levels of depth is appropriate and acceptable ignores the > question > of whether it is "necessary depth". The above example has four levels. The > first > level is not needed: > > int a = 0; > int b; > > > if ( bar == a ) > return 0; > > b = 1; > > while ( a++ < b ) { > int c = 2; // two levels > > if ( a == c ) { > return -1; > } > } > return 0; > > I don't know if there is a measure of average nesting depth but > it is certainly higher in the original version. > Well, the example was contrived mostly to show what I meant by 4, but the actual number is subjective I suppose. Maybe we should write a script to determine the nesting level in use and go from there...
> In the original example, the flow is pretty contorted to me with > the goto resulting in two returns back to back. > Yes, I think a goto should only be used to consolidate cleanup code. > Ironically, the example also raises three new style questions: > > + It declares new variables in inner scopes which has been avoided > in the past. I think this was not supported in older C standards and thus > there was no choice but to avoid it. I don't remember when it got added > to C. I assume C99 since that is our target language. But we never > discussed it. > I believe I have seen this creeping into RTEMS recently. The coding conventions say to use ANSI C, which is a vague description that could mean C99, or C90. > + And there is the question of whether variables should be initialized > when declared or not. I think historically this has been done only for > simple RHS values which are constant so as not to impose order > implications on the declarations. > Yes I think we discussed this, and it should be added to the conventions. > + C++ style // comments. Same issue with history based on older C standard. > Yes, we should stick to /* */ comments. > I did google for indentation and nesting impacting the -ilities of code and > there > are lots of opinions. This seemed like a good pulling point but I > would cite it as authoritative: > > http://micheltriana.com/2010/12/05/writing-elegant-code-and-the-maintainability-index/ > > >> -Gedare >> >>>> -- >>>> Sebastian Huber, embedded brains GmbH >>>> >>>> Address : Dornierstr. 4, D-82178 Puchheim, Germany >>>> Phone : +49 89 189 47 41-16 >>>> Fax : +49 89 189 47 41-09 >>>> E-Mail : sebastian.hu...@embedded-brains.de >>>> PGP : Public key available on request. >>>> >>>> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. >>>> _______________________________________________ >>>> rtems-devel mailing list >>>> rtems-devel@rtems.org >>>> http://www.rtems.org/mailman/listinfo/rtems-devel > > > > -- > Joel Sherrill, Ph.D. Director of Research & Development > joel.sherr...@oarcorp.com On-Line Applications Research > Ask me about RTEMS: a free RTOS Huntsville AL 35805 > Support Available (256) 722-9985 > _______________________________________________ rtems-devel mailing list rtems-devel@rtems.org http://www.rtems.org/mailman/listinfo/rtems-devel