rtems/src/scheduler* code convention issue

[Gedare Bloom]

On Wed, May 28, 2014 at 11:43 AM, Gedare Bloom <gedare at rtems.org> wrote:
> On Wed, May 28, 2014 at 11:32 AM, Sebastian Huber
> <sebastian.huber at 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.
-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.huber at 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 at rtems.org
>> http://www.rtems.org/mailman/listinfo/rtems-devel