rtems/src/scheduler* code convention issue

[Joel Sherrill]

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"
Still no attempt at a community discussion.
>> With no community discussion, this implies that one person can
>> make decisions for the community. This was a serious decision and
>> the thread consisted of 3 emails with the first being me asking
>> why you didn't follow existing practice. There was no discussion
>> of changing style, how existing code would be impacted, inconsistency
>> addressed, etc..
>>
>> That doesn't sound like a healthy discussion to change 25 years
>> of coding practice -- written style guide or not, you can look at the
>> code and see it. There are almost 200 API calls which take object ID
>> and have the _Object_Get() switch. Those are pretty damned
>> obvious ignoring any other code.
> No, looking at a random set of files to guess the "existing practice" is not an 
> option.  My point is that if you want me to follow a certain rule, then we have 
> to add this rule to the coding style.  I have no problem with a rule like 
> "avoidance of deep nesting by using early returns".  I just wanted to highlight 
> that there are some standards that use different rules.
I won't argue the point that this wasn't documented. When asked,
I and other folks have tried to document existing practice. This is an
ongoing effort and something any code base that moves from research
to production has to face. It is a similar issue to have a requirements
set. Research code simply doesn't have DO-178 Level A documentation.

FWIW a style rule for safety critical code I think is worth consideration
in some parts of the code base relates to the complexity of expressions
in if's.  Compound expressions are harder to do full path coverage on.
Some of the coding guides have rules on this. But I would never force
this on the community without discussion and an open source way to
enforce it.

I am also convinced that any attempt to enforce whatever rules we
adopt from MISRA, ESA, JSF, NASA, etc. need to be automatically
enforced and a subset of the tree deemed "critical" and be enforced.

>> Inconsistent coding is an indication that we are not a community.
>> This detracts from the code base.  Were you volunteering to
>> reformat the code to follow the new rule?
> I would not reformat unless this can be done by a tool.
>
Then we are stuck with inconsistent code or documenting
and following the existing rules.

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