rtems/src/scheduler* code convention issue

Tue May 20 16:04:01 UTC 2014

On Tue, May 20, 2014 at 11:30 AM, Joel Sherrill
<joel.sherrill at oarcorp.com> 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"
> Still no attempt at a community discussion.
Style issues like this have been discussed on this mailing list, and
the wiki page attempts to codify the rules in practice and as we the
community want them to be. The fact is that there is no reference to
nesting levels in those rules. I think 3 is a reasonable level of
nesting.

There also is nothing about having a single point of return. I don't
think we want to enforce a single point of return, but I'm not going
to complain about such code either, unless it really is horrendously
nested.

>>> 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.
>
Yes, this is on-going. So let's focus on determining the rules we want
to follow, and adding them to our conventions.

> 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.
>
We need to have a discussion about what we want our style guide to do
for us. Make coverage easier? Improve readability? Reduce source lines
of code? These priorities may be conflicting in some cases.

> 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.
>
We have had this discussion before:
http://www.rtems.org/pipermail/rtems-devel/2012-December/002159.html

>>> 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
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel