[ofa-general] [PATCH] opensm: avoid memory leaks on config parameters reloading

Sasha Khapyorsky sashak at voltaire.com
Sat Feb 7 01:48:10 PST 2009


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 <sashak at voltaire.com>
---

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




More information about the general mailing list