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.
From: sford@xxxxxxxxxxxxx
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, ×tamp); 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[] = {
- Follow-Ups:
- Re: [Ethereal-dev] patches for memory leaks
- From: sford
- Re: [Ethereal-dev] patches for memory leaks
- From: Guy Harris
- Re: [Ethereal-dev] patches for memory leaks
- Prev by Date: RE: [Ethereal-dev] [PATCH]Typo in packet-wsp.c
- Next by Date: Re: [Ethereal-dev] reorganizing source tree
- Previous by thread: RE: [Ethereal-dev] SQL patch
- Next by thread: Re: [Ethereal-dev] patches for memory leaks
- Index(es):