[PATCH 3/5] Move the memory allocation of entry buffers to a separate function

Lubomir I. Ivanov neolit123 at gmail.com
Sun Dec 23 17:53:26 PST 2012


From: "Lubomir I. Ivanov" <neolit123 at gmail.com>

parse-xml.c:
Once an entry is about to be parsed to internal data (in entry()),
we allocate a buffer and then pass it to the try_to_fill_* functions,
which take care of the "name-matching". However this method makes it
(notoriously) difficult for memory debuggers (such as dr.memory and valgrind)
to find the actual type of memory leak (e.g. is it event or divetrip related),
as the allocation preceeds the sorting.

If we move the malloc() call itself into a separate function, which is then
called from try_to_fill_*, suddenly the type of the memory leak becomes
easily determinable from the backtrace.

Signed-off-by: Lubomir I. Ivanov <neolit123 at gmail.com>
---
 parse-xml.c | 49 +++++++++++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/parse-xml.c b/parse-xml.c
index f68474a..b7de1d5 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -19,6 +19,20 @@ int verbose;
 
 struct dive_table dive_table;
 
+/* we use this as a separate function and call it from try_to_fill_*,
+ * so that memory leaks related to the parser can be debugged more easily.
+ * once a leak is detected at this location, the call stack will also indicate
+ * a try_to_fill_* function. */
+static char *alloc_fill_buffer(int size, const char* raw)
+{
+	char *buf = malloc(size + 1);
+	if (!buf)
+		exit(1);
+	memcpy(buf, raw, size);
+	buf[size] = 0;
+	return buf;
+}
+
 /*
  * Add a dive into the dive_table array
  */
@@ -592,10 +606,10 @@ static void eventtime(char *buffer, void *_duration)
 		duration->seconds += cur_sample->time.seconds;
 }
 
-static void try_to_fill_event(const char *name, char *buf)
+static void try_to_fill_event(const char *name, int size, const char *raw)
 {
+	char *buf = alloc_fill_buffer(size, raw);
 	int len = strlen(name);
-
 	start_match("event", name, buf);
 	if (MATCH(".event", utf8_string, &cur_event.name))
 		return;
@@ -613,10 +627,10 @@ static void try_to_fill_event(const char *name, char *buf)
 }
 
 /* We're in the top-level dive xml. Try to convert whatever value to a dive value */
-static void try_to_fill_dc(struct divecomputer *dc, const char *name, char *buf)
+static void try_to_fill_dc(struct divecomputer *dc, const char *name, int size, const char *raw)
 {
+	char *buf = alloc_fill_buffer(size, raw);
 	int len = strlen(name);
-
 	start_match("divecomputer", name, buf);
 
 	if (MATCH(".date", divedate, &dc->when))
@@ -634,8 +648,9 @@ static void try_to_fill_dc(struct divecomputer *dc, const char *name, char *buf)
 }
 
 /* We're in samples - try to convert the random xml value to something useful */
-static void try_to_fill_sample(struct sample *sample, const char *name, char *buf)
+static void try_to_fill_sample(struct sample *sample, const char *name, int size, const char *raw)
 {
+	char *buf = alloc_fill_buffer(size, raw);
 	int len = strlen(name);
 
 	start_match("sample", name, buf);
@@ -837,8 +852,9 @@ static void gps_location(char *buffer, void *_dive)
 }
 
 /* We're in the top-level dive xml. Try to convert whatever value to a dive value */
-static void try_to_fill_dive(struct dive *dive, const char *name, char *buf)
+static void try_to_fill_dive(struct dive *dive, const char *name, int size, const char *raw)
 {
+	char *buf = alloc_fill_buffer(size, raw);
 	int len = strlen(name);
 
 	start_match("dive", name, buf);
@@ -945,10 +961,10 @@ static void try_to_fill_dive(struct dive *dive, const char *name, char *buf)
 }
 
 /* We're in the top-level trip xml. Try to convert whatever value to a trip value */
-static void try_to_fill_trip(dive_trip_t **dive_trip_p, const char *name, char *buf)
+static void try_to_fill_trip(dive_trip_t **dive_trip_p, const char *name, int size, const char *raw)
 {
+	char *buf = alloc_fill_buffer(size, raw);
 	int len = strlen(name);
-
 	start_match("trip", name, buf);
 
 	dive_trip_t *dive_trip = *dive_trip_p;
@@ -1134,33 +1150,26 @@ static void divecomputer_end(void)
 
 static void entry(const char *name, int size, const char *raw)
 {
-	char *buf = malloc(size+1);
-
-	if (!buf)
-		return;
-	memcpy(buf, raw, size);
-	buf[size] = 0;
 	if (cur_event.active) {
-		try_to_fill_event(name, buf);
+		try_to_fill_event(name, size, raw);
 		return;
 	}
 	if (cur_sample) {
-		try_to_fill_sample(cur_sample, name, buf);
+		try_to_fill_sample(cur_sample, name, size, raw);
 		return;
 	}
 	if (cur_dc) {
-		try_to_fill_dc(cur_dc, name, buf);
+		try_to_fill_dc(cur_dc, name, size, raw);
 		return;
 	}
 	if (cur_dive) {
-		try_to_fill_dive(cur_dive, name, buf);
+		try_to_fill_dive(cur_dive, name, size, raw);
 		return;
 	}
 	if (cur_trip) {
-		try_to_fill_trip(&cur_trip, name, buf);
+		try_to_fill_trip(&cur_trip, name, size, raw);
 		return;
 	}
-	free(buf);
 }
 
 static const char *nodename(xmlNode *node, char *buf, int len)
-- 
1.7.11.msysgit.0



More information about the subsurface mailing list