rtems/src/scheduler* code convention issue
Joel Sherrill
joel.sherrill at OARcorp.com
Wed May 28 16:12:00 UTC 2014
On 5/28/2014 10:44 AM, Gedare Bloom wrote:
> 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.
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.
In the original example, the flow is pretty contorted to me with
the goto resulting in two returns back to back.
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.
+ 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.
+ C++ style // comments. Same issue with history based on older C standard.
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.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
--
Joel Sherrill, Ph.D. Director of Research & Development
joel.sherrill at OARcorp.com On-Line Applications Research
Ask me about RTEMS: a free RTOS Huntsville AL 35805
Support Available (256) 722-9985
More information about the devel
mailing list