[Bug 1656] GSoC AIO patch

bugzilla-daemon at rtems.org bugzilla-daemon at rtems.org
Sun Aug 8 15:10:14 UTC 2010


--- Comment #2 from Joel Sherrill <joel.sherrill at oarcorp.com> 2010-08-08 10:10:13 CDT ---
Comments in no order..

+ Patch should NOT include CVS subdirectory.

+ No more than one blank line at a time.

+ Don't have this


OR this


in a comment block

+ Naming violations.. get_elem is a good example.  Do not abbreviate and
it is not static (don't know whether it can be or not) and doesn't include
aio on the front.  All methods need an indication of their subsystem.

+ No test code

+ No confdefs.h changes.  There may be resources needed when you want
aio at all (e.g. CONFIGURE_AIO_ENABLE).  There may have to be some per
file you want to do AIO on (e.g. CONFIGURE_MAXIMUM_AIO_FILES).

+ aio_testinit is either in the wrong place or horribly misnamed.

+ rtems_chn is a bad name.. doesn't reflect purpose.. Internal routines
and structures for AIO probably could be named _AIO_ or AIO_ depending
on whether they are methods or structures/constants.

+ there are some alignment issues with parameters and structure elements.

+ Can't compile with warnings.  

I caught all this on a visual review of the diff.  Did not try to compile.

Please address and resubmit.

Configure bugmail: https://www.rtems.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

More information about the bugs mailing list