Ethereal-dev: [Ethereal-dev] patches for memory leaks

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

Date: Tue, 28 Oct 2003 16:07:34 -0800 (PST)
I found two minor memory leaks using "valgrind" (a good malloc debugging
tool; see http://developer.kde.org/~sewardj/ for info).

The tool currently tells me that there aren't any more on-going leaks.
Yet, the longer it runs (with a read filter), the more memory it
consumes.  Does anybody have any idea why?  Does the read filter
matching code generate a linked list of structures - parse trees
perhaps?  Something that doesn't get cleaned up with: epan_dissect_free()
and clear_fdata() that is...

Any clues?

Here's the diff -u output for files packet-diameter.c and packet-snmp.c:


--- ethereal-0.9.15/packet-diameter.c	Wed Jul  9 20:20:07 2003
+++ packet-diameter.c	Tue Oct 28 17:41:48 2003
@@ -1746,6 +1746,7 @@
 void
 proto_register_diameter(void)
 {
+	gchar *malloced_ptr;
 	static hf_register_info hf[] = {
 		{ &hf_diameter_version,
 		  { "Version", "diameter.version", FT_UINT8, BASE_HEX, NULL, 0x00,
@@ -1893,11 +1894,17 @@
 	 */
 	if (! gbl_diameterDictionary)
 		gbl_diameterDictionary = get_datafile_path(DICT_FN);
+	/* The prefs_register_string_preference() function overwrites its
+	 * "val" pointer with a newly-malloced copy of its value.  Since
+	 * gbl_diameterDictionary is always malloced, we need to free it
+	 * after registering it.  (fixed memory leak - SDF) */
+	malloced_ptr = gbl_diameterDictionary;
 	/* Now register its preferences so it can be changed. */
 	prefs_register_string_preference(diameter_module, "dictionary.name",
 									 "Diameter XML Dictionary",
 									 "Set the dictionary used for Diameter messages",
 									 &gbl_diameterDictionary);
+	g_free(malloced_ptr);
 
 	/* Desegmentation */
 	prefs_register_bool_preference(diameter_module, "desegment",

-------------------------------------------------------------------------------

--- ethereal-0.9.15/packet-snmp.c	Mon Sep  8 20:22:23 2003
+++ packet-snmp.c	Tue Oct 28 17:36:48 2003
@@ -1002,14 +1002,14 @@
 
 	guint32 error_index;
 
-	char *pdu_type_string;
+	char *pdu_type_string = NULL;
 
-	subid_t *enterprise;
+	subid_t *enterprise = NULL;
 	guint enterprise_length;
 
 	guint32 agent_ipaddr;
 
-	guint8 *agent_address;
+	guint8 *agent_address = NULL;
 	guint agent_address_length;
 
 	guint32 trap_type;
@@ -1019,13 +1019,13 @@
 	guint timestamp;
 	guint timestamp_length;
 
-	gchar *oid_string;
+	gchar *oid_string = NULL;
 
 	guint variable_bindings_length;
 
 	int vb_index;
 	guint variable_length;
-	subid_t *variable_oid;
+	subid_t *variable_oid = NULL;
 	guint variable_oid_length;
 
 	int ret;
@@ -1058,7 +1058,7 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "request ID", ret);
-			return;
+			goto func_rtn;
 		}
 		if (tree) {
 			proto_tree_add_uint(tree, hf_snmp_request_id,
@@ -1073,7 +1073,7 @@
 			    (pdu_type == SNMP_MSG_GETBULK) ? "non-repeaters"
 			    				   : "error status",
 			    ret);
-			return;
+			goto func_rtn;
 		}
 		if (tree) {
 			if (pdu_type == SNMP_MSG_GETBULK) {
@@ -1095,7 +1095,7 @@
 			    (pdu_type == SNMP_MSG_GETBULK) ? "max repetitions"
 			    				   : "error index",
 			    ret);
-			return;
+			goto func_rtn;
 		}
 		if (tree) {
 			if (pdu_type == SNMP_MSG_GETBULK) {
@@ -1116,15 +1116,17 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "enterprise OID", ret);
-			return;
+			goto func_rtn;
 		}
 		if (tree) {
 			oid_string = format_oid(enterprise, enterprise_length);
 			proto_tree_add_string(tree, hf_snmp_enterprise, tvb,
 			    offset, length, oid_string);
 			g_free(oid_string);
+			oid_string = NULL;
 		}
 		g_free(enterprise);
+		enterprise = NULL;
 		offset += length;
 
 		/* agent address */
@@ -1134,7 +1136,7 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "agent address", ret);
-			return;
+			goto func_rtn;
 		}
 		if (!((cls == ASN1_APL && con == ASN1_PRI && tag == SNMP_IPA) ||
 		    (cls == ASN1_UNI && con == ASN1_PRI && tag == ASN1_OTS))) {
@@ -1142,19 +1144,19 @@
 			   Banyan" */
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "agent_address", ASN1_ERR_WRONG_TYPE);
-			return;
+			goto func_rtn;
 		}
 		if (agent_address_length != 4) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "agent_address", ASN1_ERR_WRONG_LENGTH_FOR_TYPE);
-			return;
+			goto func_rtn;
 		}
 		ret = asn1_string_value_decode (&asn1,
 		    agent_address_length, &agent_address);
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "agent address", ret);
-			return;
+			goto func_rtn;
 		}
 		length = asn1.offset - start;
 		if (tree) {
@@ -1171,6 +1173,7 @@
 			}
 		}
 		g_free(agent_address);
+		agent_address = NULL;
 		offset += length;
 
 	        /* generic trap type */
@@ -1178,7 +1181,7 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "generic trap type", ret);
-			return;
+			goto func_rtn;
 		}
 		if (tree) {
 			proto_tree_add_uint(tree, hf_snmp_traptype, tvb,
@@ -1191,7 +1194,7 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "specific trap type", ret);
-			return;
+			goto func_rtn;
 		}
 		if (tree) {
 			proto_tree_add_uint(tree, hf_snmp_spectraptype, tvb,
@@ -1206,20 +1209,20 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "timestamp", ret);
-			return;
+			goto func_rtn;
 		}
 		if (!((cls == ASN1_APL && con == ASN1_PRI && tag == SNMP_TIT) ||
 		    (cls == ASN1_UNI && con == ASN1_PRI && tag == ASN1_INT))) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "timestamp", ASN1_ERR_WRONG_TYPE);
-			return;
+			goto func_rtn;
 		}
 		ret = asn1_uint32_value_decode(&asn1, timestamp_length,
 		    &timestamp);
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "timestamp", ret);
-			return;
+			goto func_rtn;
 		}
 		length = asn1.offset - start;
 		if (tree) {
@@ -1236,7 +1239,7 @@
 	if (ret != ASN1_ERR_NOERROR) {
 		dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			"variable bindings header", ret);
-		return;
+		goto func_rtn;
 	}
 	offset += length;
 
@@ -1251,7 +1254,7 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 				"variable binding header", ret);
-			return;
+			goto func_rtn;
 		}
 		sequence_length += length;
 
@@ -1261,7 +1264,7 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "variable binding OID", ret);
-			return;
+			goto func_rtn;
 		}
 		sequence_length += length;
 
@@ -1320,11 +1323,18 @@
 		if (ret != ASN1_ERR_NOERROR) {
 			dissect_snmp_parse_error(tvb, offset, pinfo, tree,
 			    "variable", ret);
-			return;
+			goto func_rtn;
 		}
 		offset += length;
 		variable_bindings_length -= length;
 	}
+
+	/* Added to fix memory leaks - SDF */
+func_rtn:
+	if (variable_oid != NULL) g_free(variable_oid);  variable_oid = NULL;
+	if (enterprise != NULL) g_free(enterprise);  enterprise = NULL;
+	if (agent_address != NULL) g_free(agent_address);  agent_address = NULL;
+	if (oid_string != NULL) g_free(oid_string);  oid_string = NULL;
 }
 
 static const value_string qos_vals[] = {