When OpenSM string config parameters are loaded it will always allocate
memory (except NULL value), and will free and reallocate on reloading.

Signed-off-by: Sasha Khapyorsky <[email protected]>
---

On 14:24 Tue 03 Feb     , Sasha Khapyorsky wrote:
> 
> I'm applying this with several changes:
> 
> - disable update option and setup function for all string parameter -
>   as I commented originally opts_parse_charp() will leak memory and this
>   cannot be ignored if config file is rescanned. Exception is QoS string
>   parameters where memory leak is handled.

This probably solves an issue with potential memory leaks....

 opensm/opensm/main.c       |   33 +++++++++++++++-----------
 opensm/opensm/osm_subnet.c |   55 +++++++++++++------------------------------
 2 files changed, 36 insertions(+), 52 deletions(-)

diff --git a/opensm/opensm/main.c b/opensm/opensm/main.c
index c09a54e..a8dc9e6 100644
--- a/opensm/opensm/main.c
+++ b/opensm/opensm/main.c
@@ -507,6 +507,11 @@ int osm_manager_loop(osm_subn_opt_t * p_opt, osm_opensm_t 
* p_osm)
 
 /**********************************************************************
  **********************************************************************/
+#define SET_STR_OPT(opt, val) do { \
+       if (opt) free(opt); \
+       opt = val ? strdup(val) : NULL ; \
+} while (0)
+
 int main(int argc, char *argv[])
 {
        osm_opensm_t osm;
@@ -650,7 +655,7 @@ int main(int argc, char *argv[])
                        /*
                           Specifies ignore guids file.
                         */
-                       opt.port_prof_ignore_file = optarg;
+                       SET_STR_OPT(opt.port_prof_ignore_file, optarg);
                        printf(" Ignore Guids File = %s\n",
                               opt.port_prof_ignore_file);
                        break;
@@ -706,7 +711,7 @@ int main(int argc, char *argv[])
                            || strcmp(optarg, OSM_LOOPBACK_CONSOLE) == 0
 #endif
                            )
-                               opt.console = optarg;
+                               SET_STR_OPT(opt.console, optarg);
                        else
                                printf("-console %s option not understood\n",
                                       optarg);
@@ -763,7 +768,7 @@ int main(int argc, char *argv[])
                        break;
 
                case 'f':
-                       opt.log_file = optarg;
+                       SET_STR_OPT(opt.log_file, optarg);
                        break;
 
                case 'L':
@@ -778,7 +783,7 @@ int main(int argc, char *argv[])
                        break;
 
                case 'P':
-                       opt.partition_config_file = optarg;
+                       SET_STR_OPT(opt.partition_config_file, optarg);
                        break;
 
                case 'N':
@@ -790,7 +795,7 @@ int main(int argc, char *argv[])
                        break;
 
                case 'Y':
-                       opt.qos_policy_file = optarg;
+                       SET_STR_OPT(opt.qos_policy_file, optarg);
                        printf(" QoS policy file \'%s\'\n", optarg);
                        break;
 
@@ -829,7 +834,7 @@ int main(int argc, char *argv[])
                        break;
 
                case 'R':
-                       opt.routing_engine_names = optarg;
+                       SET_STR_OPT(opt.routing_engine_names, optarg);
                        printf(" Activate \'%s\' routing engine(s)\n", optarg);
                        break;
 
@@ -844,17 +849,17 @@ int main(int argc, char *argv[])
                        break;
 
                case 'M':
-                       opt.lid_matrix_dump_file = optarg;
+                       SET_STR_OPT(opt.lid_matrix_dump_file, optarg);
                        printf(" Lid matrix dump file is \'%s\'\n", optarg);
                        break;
 
                case 'U':
-                       opt.lfts_file = optarg;
+                       SET_STR_OPT(opt.lfts_file, optarg);
                        printf(" LFTs file is \'%s\'\n", optarg);
                        break;
 
                case 'S':
-                       opt.sa_db_file = optarg;
+                       SET_STR_OPT(opt.sa_db_file, optarg);
                        printf(" SA DB file is \'%s\'\n", optarg);
                        break;
 
@@ -862,7 +867,7 @@ int main(int argc, char *argv[])
                        /*
                           Specifies root guids file
                         */
-                       opt.root_guid_file = optarg;
+                       SET_STR_OPT(opt.root_guid_file, optarg);
                        printf(" Root Guid File: %s\n", opt.root_guid_file);
                        break;
 
@@ -870,20 +875,20 @@ int main(int argc, char *argv[])
                        /*
                           Specifies compute node guids file
                         */
-                       opt.cn_guid_file = optarg;
+                       SET_STR_OPT(opt.cn_guid_file, optarg);
                        printf(" Compute Node Guid File: %s\n",
                               opt.cn_guid_file);
                        break;
 
                case 'm':
                        /* Specifies ids guid file */
-                       opt.ids_guid_file = optarg;
+                       SET_STR_OPT(opt.ids_guid_file, optarg);
                        printf(" IDs Guid File: %s\n", opt.ids_guid_file);
                        break;
 
                case 'X':
                        /* Specifies guid routing order file */
-                       opt.guid_routing_order_file = optarg;
+                       SET_STR_OPT(opt.guid_routing_order_file, optarg);
                        printf(" GUID Routing Order File: %s\n", 
opt.guid_routing_order_file);
                        break;
 
@@ -912,7 +917,7 @@ int main(int argc, char *argv[])
 #endif                         /* ENABLE_OSM_PERF_MGR */
 
                case 3:
-                       opt.prefix_routes_file = optarg;
+                       SET_STR_OPT(opt.prefix_routes_file, optarg);
                        break;
                case 4:
                        opt.consolidate_ipv6_snm_req = TRUE;
diff --git a/opensm/opensm/osm_subnet.c b/opensm/opensm/osm_subnet.c
index bd52f76..42c5682 100644
--- a/opensm/opensm/osm_subnet.c
+++ b/opensm/opensm/osm_subnet.c
@@ -488,21 +488,15 @@ static void subn_init_qos_options(IN osm_qos_options_t * 
opt)
 {
        opt->max_vls = 0;
        opt->high_limit = -1;
-       opt->vlarb_high = NULL;
-       opt->vlarb_low = NULL;
-       opt->sl2vl = NULL;
-}
-
-static void subn_free_qos_options(IN osm_qos_options_t * opt)
-{
        if (opt->vlarb_high)
                free(opt->vlarb_high);
-
+       opt->vlarb_high = NULL;
        if (opt->vlarb_low)
                free(opt->vlarb_low);
-
+       opt->vlarb_low = NULL;
        if (opt->sl2vl)
                free(opt->sl2vl);
+       opt->sl2vl = NULL;
 }
 
 /**********************************************************************
@@ -518,7 +512,7 @@ void osm_subn_set_default_opt(IN osm_subn_opt_t * const 
p_opt)
        p_opt->m_key_lease_period = 0;
        p_opt->sweep_interval = OSM_DEFAULT_SWEEP_INTERVAL_SECS;
        p_opt->max_wire_smps = OSM_DEFAULT_SMP_MAX_ON_WIRE;
-       p_opt->console = OSM_DEFAULT_CONSOLE;
+       p_opt->console = strdup(OSM_DEFAULT_CONSOLE);
        p_opt->console_port = OSM_DEFAULT_CONSOLE_PORT;
        p_opt->transaction_timeout = OSM_DEFAULT_TRANS_TIMEOUT_MILLISEC;
        /* by default we will consider waiting for 50x transaction timeout 
normal */
@@ -566,13 +560,13 @@ void osm_subn_set_default_opt(IN osm_subn_opt_t * const 
p_opt)
        p_opt->dump_files_dir = getenv("OSM_TMP_DIR");
        if (!p_opt->dump_files_dir || !(*p_opt->dump_files_dir))
                p_opt->dump_files_dir = OSM_DEFAULT_TMP_DIR;
-
-       p_opt->log_file = OSM_DEFAULT_LOG_FILE;
+       p_opt->dump_files_dir = strdup(p_opt->dump_files_dir);
+       p_opt->log_file = strdup(OSM_DEFAULT_LOG_FILE);
        p_opt->log_max_size = 0;
-       p_opt->partition_config_file = OSM_DEFAULT_PARTITION_CONFIG_FILE;
+       p_opt->partition_config_file = 
strdup(OSM_DEFAULT_PARTITION_CONFIG_FILE);
        p_opt->no_partition_enforcement = FALSE;
        p_opt->qos = FALSE;
-       p_opt->qos_policy_file = OSM_DEFAULT_QOS_POLICY_FILE;
+       p_opt->qos_policy_file = strdup(OSM_DEFAULT_QOS_POLICY_FILE);
        p_opt->accum_log_file = TRUE;
        p_opt->port_prof_ignore_file = NULL;
        p_opt->port_profile_switch_nodes = FALSE;
@@ -591,7 +585,7 @@ void osm_subn_set_default_opt(IN osm_subn_opt_t * const 
p_opt)
        p_opt->exit_on_fatal = TRUE;
        p_opt->enable_quirks = FALSE;
        p_opt->no_clients_rereg = FALSE;
-       p_opt->prefix_routes_file = OSM_DEFAULT_PREFIX_ROUTES_FILE;
+       p_opt->prefix_routes_file = strdup(OSM_DEFAULT_PREFIX_ROUTES_FILE);
        p_opt->consolidate_ipv6_snm_req = FALSE;
        subn_init_qos_options(&p_opt->qos_options);
        subn_init_qos_options(&p_opt->qos_ca_options);
@@ -753,25 +747,16 @@ static void opts_parse_charp(IN osm_subn_t *p_subn, IN 
char *p_key,
        char **p_val = p_v;
        const char *current_str = *p_val ? *p_val : null_str ;
 
-       if (!p_val_str)
-               return;
-
-       if (strcmp(p_val_str, current_str)) {
+       if (p_val_str && strcmp(p_val_str, current_str)) {
+               char *new;
                log_config_value(p_key, "%s", p_val_str);
                /* special case the "(null)" string */
-               if (strcmp(null_str, p_val_str) == 0) {
-                       if (pfn)
-                               pfn(p_subn, NULL);
-                       *p_val = NULL;
-               } else {
-                       if (pfn)
-                               pfn(p_subn, p_val_str);
-                       /*
-                         Ignore the possible memory leak here;
-                         the pointer may be to a static default.
-                       */
-                       *p_val = strdup(p_val_str);
-               }
+               new = strcmp(null_str, p_val_str) ? strdup(p_val_str) : NULL;
+               if (pfn)
+                       pfn(p_subn, new);
+               if (*p_val)
+                       free(*p_val);
+               *p_val = new;
        }
 }
 
@@ -1211,12 +1196,6 @@ int osm_subn_rescan_conf_files(IN osm_subn_t * const 
p_subn)
                return -1;
        }
 
-       subn_free_qos_options(&p_opts->qos_options);
-       subn_free_qos_options(&p_opts->qos_ca_options);
-       subn_free_qos_options(&p_opts->qos_sw0_options);
-       subn_free_qos_options(&p_opts->qos_swe_options);
-       subn_free_qos_options(&p_opts->qos_rtr_options);
-
        subn_init_qos_options(&p_opts->qos_options);
        subn_init_qos_options(&p_opts->qos_ca_options);
        subn_init_qos_options(&p_opts->qos_sw0_options);
-- 
1.6.1.2.319.gbd9e

_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to