rtems/src/scheduler* code convention issue

[Gedare Bloom]

I have updated the wiki page with the recommendations Joel listed,
since they seem historical and still are relevant.
http://www.rtems.org/wiki/index.php/Coding_Conventions#Readability

-Gedare

On Wed, May 28, 2014 at 5:15 PM, Joel Sherrill
<joel.sherrill at oarcorp.com> wrote:
>
> 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
>