[PATCH] GSOC2012 project atomic support for RTEMS

[Joel Sherrill]

The first thing I noticed was that git complained about white
space issues. I have attached the output from that.

Here are random comments as I spotted them beyond that.

+ I ran all tests on powerpc/psim and the atomic tests passed.
+ Check the copyright year to 2012 everywhere.
+ rtems/score/atomic.h
     - Capitalize the first letter in sentences in comments.
     - }Atomic_Memory_barrier needs a space after }
    - Each Atomic Load, Store, etc method needs its own
      Doxygen comment block. There is a Doxygen command
      to reuse another comment (I think) which might be useful
      here.
    - line up parameter names in prototypes.
+ atomic.inl
    - Copyright and file description go in separate comment blocks
    - Doxygen file header goes first
    - Stay < 80 columns
    - acq_XXX? Can we spell that out completely, please? In RTEMS
      we do not abbreviate.
    - Ditto for "rel"
    - Ditto for "ptr"
    - Ditto on others I might have missed
    - if( should be if (
    - normally ...
          if X
             return Y
          else
             return X
       is written
           if X
              return Y
           return X
      Notice that the else is not needed and we avoid indenting more.
      This will really clean up a method like _Atomic_Fetch_add_int
    - Again no Doxygen comments on methods
    - Single line only -- no need for > 1 blank line
    - line up arguments
  + i386 file
     - spacing, abbreviation
     - watch \ at end of line to make sure they line up. They don't
  + powerpc
     - Same as i386

I didn't have as much time to spend on the test code.
    - no intermediate blank lines in output.
    - copyright year
    - no double "*" lines in comments
    - What's the purpose of TASK_NUMS? Better name?
    - Why is the CONFIGURE_MAXIMUM_TASKS not near the rest of the
      parameters? Comment it in the "odd" place and explain near the
      rest of the parameters where it is.  If it is needed to do this by the
      test design (e.g. file reuse, etc). Try to avoid it.
   - ATOMIC_STORE_NO_BARRIER > 80 columns

Please also take the time to update the Wiki page for this as a project.
It clearly has made a lot of progress but since it only supports 2 target
architectures, there is work left.

Mostly just clean up. The technical side of this got beaten mercilessly
during the summer. :)

Thanks.

--joel
On 09/18/2012 10:50 AM, yangwei weiyang wrote:
> Hi
>
> There are some patches to RTEMS which need review.
>
> It is the final version of GSOC2012 project atomic support for RTEMS.
> This series include six patches:
>
>    -The first patch includes all the modifies to the related *.am and *.ac files.
>    -The second patch includes the atomic operations API definition for
> all the architectures.
>    -The third patch includes the implementation of atomic operations
> for i386 architecture.
>    -The fourth patch includes the implementation of atomic operations
> for PowerPC architecture.
>    -The fifth patch includes seven test cases of atomic operations for UP system.
>    -The sixth patch includes seven test cases of atomic operations for
> SMP system.
>
> Any suggestion is welcome!
>


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


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: white.txt
URL: <http://lists.rtems.org/pipermail/devel/attachments/20120919/81f3371e/attachment-0001.txt>