sbgp/base: fix memory leaks

This commit fixes memory leaks discovered in the sbgp setup code. We
were leaking an opal_argv as well as some list items. I took the
opportunity to clean up the code a little which includes making use of
the opal_argv_free function.

cmr=v1.8.2:reviewer=manjugv

This commit was SVN r31745.

ompi/mca/sbgp/base/sbgp_base_frame.c

@@ -1,7 +1,8 @@
+/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
 /*
  * Copyright (c) 2009-2012 Oak Ridge National Laboratory.  All rights reserved.
  * Copyright (c) 2009-2012 Mellanox Technologies.  All rights reserved.
- * Copyright (c) 2012-2013 Los Alamos National Security, Inc.  All rights reserved. 
+ * Copyright (c) 2012-2014 Los Alamos National Security, Inc.  All rights reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -57,117 +58,71 @@ OBJ_CLASS_INSTANCE(sbgp_base_component_keyval_t,
         NULL,
         NULL);
 
-#define RELEASE_LIST_OF_STRINGS(str_arr)        \
-    do {                                        \
-        int arr_size  = 0;                      \
-        if (NULL != str_arr) {                  \
-            while(NULL != str_arr[arr_size]) {  \
-                free(str_arr[arr_size]);        \
-                ++arr_size;                     \
-            }                                   \
-            free(str_arr);                      \
-        }                                       \
-    } while (0)
-
 /* get list of subgrouping coponents to use */
 static int ompi_sbgp_set_components_to_use(opal_list_t *sbgp_components_avail,
         opal_list_t *sbgp_components_in_use)
 {
     /* local variables */
-    opal_list_item_t *item;
     const mca_base_component_t *component;
     mca_base_component_list_item_t *cli;
     sbgp_base_component_keyval_t *clj;
-    char **subgoups_requested = NULL, **sbgp_string = NULL;
+    char **subgroups_requested = NULL, **sbgp_string = NULL;
     char *sbgp_component, *sbgp_key;
     const char *component_name;
-    size_t str_len;
     int i, sbgp_size = 0,
         sbgp_string_size = 0,
         rc = OMPI_SUCCESS;
 
     /* split the list of requested subgroups */
-    subgoups_requested = opal_argv_split(ompi_sbgp_subgroups_string, ',');
+    subgroups_requested = opal_argv_split(ompi_sbgp_subgroups_string, ',');
-    if(NULL == subgoups_requested) {
+    if(NULL == subgroups_requested) {
         return OMPI_ERROR;
     }
-    /* debug print */
+    sbgp_size = opal_argv_count (subgroups_requested);
-    /*
-    fprintf(stderr,"FFF ompi_sbgp_subgroups_string %s \n",ompi_sbgp_subgroups_string);
-    fflush(stderr);
-    */
-    /* end debug */
-
-    /* count arguments - set number of levels to match the input value */
-    while(subgoups_requested[sbgp_size]){
-        ++sbgp_size;
-
-    }
-    /*
-    fprintf(stderr,"DDD subgroup size %d \n",sbgp_size);
-    fflush(stderr);
-    */
 
     /* Initialize list */
     OBJ_CONSTRUCT(sbgp_components_in_use, opal_list_t);
 
     /* loop over list of components requested */
-    for(i = 0; i < sbgp_size; i++) {
+    for (i = 0; i < sbgp_size; i++) {
-        sbgp_string_size = 0;
-        sbgp_component = NULL;
-        sbgp_key = NULL;
-
         /* get key-value */
-        sbgp_string = opal_argv_split(subgoups_requested[i], ':');
+        sbgp_string = opal_argv_split(subgroups_requested[i], ':');
-        while (sbgp_string[sbgp_string_size]) {
+        if (NULL == sbgp_string) {
-            ++sbgp_string_size;
+            rc = OMPI_ERR_OUT_OF_RESOURCE;
+            break;
         }
 
-        switch (sbgp_string_size) {
+        sbgp_string_size = opal_argv_count (sbgp_string);
-            case 2:
+        if (sbgp_string_size < 1 | sbgp_string_size > 2) {
-                sbgp_key = sbgp_string[1];
+            opal_output(ompi_sbgp_base_framework.framework_output,
-            case 1:
-                sbgp_component = sbgp_string[0];
-                break;
-            default:
-                opal_output(ompi_sbgp_base_framework.framework_output,
                         "Requested SBGP configuration is illegal %s",
-                        subgoups_requested[i]);
+                        subgroups_requested[i]);
-                RELEASE_LIST_OF_STRINGS(sbgp_string);
+            opal_argv_free (sbgp_string);
-                rc = OMPI_ERROR;
+            rc = OMPI_ERROR;
-                goto error;
+            break;
         }
-        /* loop over discovered components */
-        for (item = opal_list_get_first(sbgp_components_avail) ;
-                opal_list_get_end(sbgp_components_avail) != item;
-                item = opal_list_get_next(item)
-                ) {
 
-            cli = (mca_base_component_list_item_t *) item;
+        /* it is garanteed that sbgp_string[1] will either be NULL (count = 1) or a string */
+        sbgp_key = sbgp_string[1];
+        sbgp_component = sbgp_string[0];
+
+        /* loop over discovered components */
+	OPAL_LIST_FOREACH(cli, sbgp_components_avail, mca_base_component_list_item_t) {
             component = cli->cli_component;
             component_name = component->mca_component_name;
-        /* debug print */
-        /*
-        fprintf(stderr,"component name %s sbgp_component %s \n",component_name,sbgp_component);
-        fflush(stderr);
-        */
-        /* end debug */
-
-            str_len = strlen(component_name);
 
             /* key_value[0] has the component name, and key_value[1], if
             ** it is not NULL, has the key_value associated with this
             ** instance of the compoenent
             */
 
-            if((str_len == strlen(sbgp_component)) &&
+            if (0 == strcmp (component_name, sbgp_component)) {
-                    (0 == strncmp(component_name,sbgp_component,str_len))) {
                  /* found selected component */
                  clj = OBJ_NEW(sbgp_base_component_keyval_t);
                  if (NULL == clj) {
                      rc = OPAL_ERR_OUT_OF_RESOURCE;
-                     RELEASE_LIST_OF_STRINGS(sbgp_string);
+                     opal_argv_free (sbgp_string);
-                     goto error;
+                     goto exit_ERROR;
                  }
                  /* fprintf(stderr,"sbgp selecting %s %s\n", sbgp_component, component_name); */
 
@@ -179,10 +134,11 @@ static int ompi_sbgp_set_components_to_use(opal_list_t *sbgp_components_avail,
                  }
                  opal_list_append(sbgp_components_in_use, (opal_list_item_t *)clj);
                  break;
-             }
+            }
-         }
+        }
-         RELEASE_LIST_OF_STRINGS(sbgp_string);
+
-     }
+        opal_argv_free (sbgp_string);
+    }
 
     /* Note: Need to add error checking to make sure all requested functions
     ** were found */
@@ -190,9 +146,8 @@ static int ompi_sbgp_set_components_to_use(opal_list_t *sbgp_components_avail,
     /*
     ** release resources
     ** */
-    /* subgoups_requested */
+ exit_ERROR:
-error:
+    opal_argv_free (subgroups_requested);
-    /* RELEASE_LIST_OF_STRINGS(subgoups_requested); */
 
     return rc;
 }
@@ -213,6 +168,14 @@ static int mca_sbgp_base_register(mca_base_register_flag_t flags)
 
 static int mca_sbgp_base_close(void)
 {
+    opal_list_item_t *item;
+
+    while (NULL != (item = opal_list_remove_first (&mca_sbgp_base_components_in_use))) {
+        OBJ_RELEASE(item);
+    }
+
+    OBJ_DESTRUCT(&mca_sbgp_base_components_in_use);
+
     return mca_base_framework_components_close(&ompi_sbgp_base_framework, NULL);
 }