rtems/src/scheduler* code convention issue

[Joel Sherrill]

On 5/28/2014 2:21 PM, Gedare Bloom wrote:
> On Wed, May 28, 2014 at 12:30 PM, Sebastian Huber
> <sebastian.huber at embedded-brains.de> wrote:
>> On 2014-05-28 17:43, Gedare Bloom wrote:
>>>> 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;
>>> }
>>>
>> For the cpukit the coding style should enforce that new/changed code looks
>> similar enough to the exiting code.  I really don't care how particular
>> rules look like since you can find for every rule an arbitrary amount of
>> pros and cons.  I think however that it is very important that the rules in
>> charge are documented.  I should be possible to give newcomers a document
>> that describes the coding style in detail.  It can also be used to judge
>> code contributions.
>>
>> A "avoidance of deep nesting by using early returns" rule should cover what
>> happens if it leads to duplicated clean up code.
>>
> I agree completely. My main point is that I don't know what "deep"
> means here. You can always refactor code to reduce the depth to 1
> nesting level, but no one wants that either. I guess what is the
> prevailing sense of the nesting level we have/want?
Arbitrary depth isn't the point. It is why you got there.  If the logic
REALLY requires for levels of depth, great. But if the first two are because
you didn't use an easy case for an early return, then something went wrong.

I am sure that no matter whatever anyone states as a rule, we can find
some code that violates it. :)

But .. the guidance in the dark ages had some of this:

+ parameter checking should be done first and early outs.

+ No allocation or critical section entries until all error checking
   that could be done otherwise had been finished.

+ Test and action should stay close together. Visually you should
see action and reaction. Makes code easier to understand.

These generally meant that there you were still at one level
of indentation when "real work started".  And parameters could
be trusted. This is usually where the "id -> ptr switch" starts.

+ Situations vary but, in general, enter critical sections and then
  do error checking which requires locks. Release locks and bail
  out early if they fail.

+ I think we generally do no more than two allocation operations
   in the "OS proper". One to allocate an object instance and another
   to allocate some additional memory like for for tasks and
   message queues. This simplifies clean up.

Use of a goto required arguments and justifications. They were very
much  frowned upon.

We also tried to write code which was simple and didn't expect
much in the way of optimization. I don't know directly how to
show an example of this.

Another odd one that is value based is to name variables
and write conditions  so the expressions in if's make sense
when read. If the name and value make it hard to understand,
you likely made a mistake. Sometimes it is poor name choice
but in others, it is just bad logic representation. Not a great
example, but "if car not in park and not in neutral and
not in reverse" is not as obvious as "if car is in drive"

We write code once but read it many times. There is no such
thing as code which is too obvious for comments. Readability
and understandability are critical.

The newer suggestions (hard to say rules on these) which may not
be followed as well are

+ to  avoid complex logical expressions in ifs, fors, and whiles.
These tend to be hard to get coverage for. It is often hard to
even trip the entire expression. 

+ avoid inlining methods with complicated logic. This results
in branch explosion when testing.

+ try to avoid duplicating code and use a method.

+ Debug assertions are nice.

I think these -- even the suggestions --  are generally followed in
score, sapi, rtems, posix and probably most of libcsupport.
Other directories would have to be examined on a case by case basis.

And... Third party code is not modified.

== Random rant on code width

We originally aimed for 1 printed page per function. This is where the
79 columns came from. The length is rather arbitrary. In general
it was from the first line of "real code". When we got off VT100's
and moved to Xterms, it roughly matched the length of an Xterm
from start of method. Which generally is about 50-ish lines or a page.

If you asked me a few years ago, I might have been argued that the
width was arbitrary and old. I admit to feeling old on this one. But
a few factors influence me to hold on to this one

- The code should look good for EVERYONE under some standard
width assumptions. Just because you have 1900+ pixels of widths,
doesn't mean it is smart to write code that wide.  Where it starts
to wrap should be the same for all persons reading the code.

- Helping Michele publish a couple of books. There are guidelines
for printed text to enhance understandability. If you look at any
book, you will notice that there are fairly normal line lengths.
There are even US FDA regulations for prescription information
based on those. Code is written to be read. Let's treat it like
we would literature.

 - "Printing" is still a real requirement. We will eventually have to
generate printed coverage reports with code and the width of
the code is an issue for that.

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

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