rtems/src/scheduler* code convention issue

[Gedare Bloom]

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;
}

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