rtems/src/scheduler* code convention issue
Gedare Bloom
gedare at rtems.org
Wed May 28 19:19:29 UTC 2014
On Wed, May 28, 2014 at 12:12 PM, Joel Sherrill
<joel.sherrill at oarcorp.com> wrote:
> 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.
>
Well, the example was contrived mostly to show what I meant by 4, but
the actual number is subjective I suppose. Maybe we should write a
script to determine the nesting level in use and go from there...
> In the original example, the flow is pretty contorted to me with
> the goto resulting in two returns back to back.
>
Yes, I think a goto should only be used to consolidate cleanup code.
> 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.
>
I believe I have seen this creeping into RTEMS recently. The coding
conventions say to use ANSI C, which is a vague description that could
mean C99, or C90.
> + 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.
>
Yes I think we discussed this, and it should be added to the conventions.
> + C++ style // comments. Same issue with history based on older C standard.
>
Yes, we should stick to /* */ comments.
> 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