coll/libnbc: rewrite parts of libnbc

This commit rewrites parts of libnbc to fix issues identified by
coverity and myself. The changes are as follows:

 - libnbc function would return invalid error codes (internal to
   libnbc) to the mpi layer. These codes names are of the form
   NBC_. They do not match up with the error codes expected by the mpi
   layer. I purged the use of all these error codes with the exception
   of NBC_OK and NBC_CONTINUE in progress. These codes are used to
   identify when a request handle is complete.

 - Handles and schedules were leaked by all collective routines on
   error. A new routine was added to return a collective handle
   (NBC_Return_handle).

 - Temporary buffers containting in/out neighbors for neighborhood
   collectives were always leaked.

 - Neigborhood collectives contained code to handle MPI_IN_PLACE which
   is never a valid input for the send or receive buffer. Stipped this
   code out.

 - Files were inconsistently named. Most are nbc_isomething.c but one
   was named coll_libnbc_ireduce_scatter_block.c.

 - Made the NBC_Schedule "structure" and object so it can be
   retained/released. This may enable the use of schedule caching at a
   later time. More testing will be needed to ensure the caching code
   works. If it doesn't the code should be stripped out completely.

 - Added code to simply common case of scheduling send/recv +
   barrier.

 - Code cleanup for readability.

The code now passes the clang static analyzer.

Signed-off-by: Nathan Hjelm <[email protected]>

Author: hjelmn

ompi/mca/coll/libnbc/Makefile.am

@@ -22,7 +22,6 @@
 sources = \
         coll_libnbc.h \
 	coll_libnbc_component.c \
-	coll_libnbc_ireduce_scatter_block.c \
 	nbc.c \
 	nbc_internal.h \
 	libdict/dict.h \
@@ -49,6 +48,7 @@ sources = \
 	nbc_ineighbor_alltoallw.c \
 	nbc_ireduce.c \
 	nbc_ireduce_scatter.c \
+	nbc_ireduce_scatter_block.c \
 	nbc_iscan.c \
 	nbc_iscatter.c \
 	nbc_iscatterv.c \

ompi/mca/coll/libnbc/coll_libnbc.h

@@ -98,9 +98,16 @@ OBJ_CLASS_DECLARATION(ompi_coll_libnbc_module_t);
 
 typedef ompi_coll_libnbc_module_t NBC_Comminfo;
 
-/* a schedule is basically a pointer to some memory location where the
- * schedule array resides */
-typedef void* NBC_Schedule;
+struct NBC_Schedule {
+    opal_object_t super;
+    volatile int size;
+    volatile int current_round_offset;
+    char *data;
+};
+
+typedef struct NBC_Schedule NBC_Schedule;
+
+OBJ_CLASS_DECLARATION(NBC_Schedule);
 
 struct ompi_coll_libnbc_request_t {
     ompi_request_t super;
@@ -110,7 +117,7 @@ struct ompi_coll_libnbc_request_t {
     volatile int req_count;
     ompi_request_t **req_array;
     NBC_Comminfo *comminfo;
-    volatile NBC_Schedule *schedule;
+    NBC_Schedule *schedule;
     void *tmpbuf; /* temporary buffer e.g. used for Reduce */
     /* TODO: we should make a handle pointer to a state later (that the user
      * can move request handles) */
@@ -134,9 +141,9 @@ typedef ompi_coll_libnbc_request_t NBC_Handle;
 
 #define OMPI_COLL_LIBNBC_REQUEST_RETURN(req)                            \
     do {                                                                \
-        OMPI_REQUEST_FINI(&request->super);                             \
+        OMPI_REQUEST_FINI(&(req)->super);                               \
         opal_free_list_return (&mca_coll_libnbc_component.requests,     \
-                               (opal_free_list_item_t*) req);           \
+                               (opal_free_list_item_t*) (req));         \
     } while (0)
 
 int ompi_coll_libnbc_progress(void);

ompi/mca/coll/libnbc/coll_libnbc_component.c

@@ -239,7 +239,7 @@ ompi_coll_libnbc_progress(void)
 
     OPAL_LIST_FOREACH_SAFE(request, next, &mca_coll_libnbc_component.active_requests,
                            ompi_coll_libnbc_request_t) {
-        if (NBC_OK == NBC_Progress(request)) {
+        if (OMPI_SUCCESS == NBC_Progress(request)) {
             /* done, remove and complete */
             opal_list_remove_item(&mca_coll_libnbc_component.active_requests,
                                   &request->super.super.super);