Skip to contentSkip to conversation

Clean up all memory leaks

PATCH In review Alan D. Brunelle 2 participants 4 messages Apr 9, 2007 – Apr 10, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Clean up all memory leaks

From Alan D. Brunelle <[email protected]>

Ensure that all allocated memory was released.

Lots of minor clean up to help ensure all allocated memory is freed prior to exit. (Also closed a few files that were left open.)

# valgrind --leak-check=yes --show-reachable=yes btt -i bp.bin -o btt -v
==21322== Memcheck, a memory error detector.
==21322== Copyright (C) 2002-2006, and GNU GPL'd, by Julian Seward et al.
==21322== Using LibVEX rev 1658, a library for dynamic binary translation.
==21322== Copyright (C) 2004-2006, and GNU GPL'd, by OpenWorks LLP.
==21322== Using valgrind-3.2.1-Debian, a dynamic binary instrumentation framework.
==21322== Copyright (C) 2000-2006, and GNU GPL'd, by Julian Seward et al.
==21322== For more details, rerun with: -v
==21322== 
Sending range data to btt.dat
Sending stats data to btt.avg
   4581291 traces @ 10.7 Ktps in 426.578804 seconds                             	tree = |0|
   ==21322== 
   ==21322== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 8 from 1)
   ==21322== malloc/free: in use at exit: 0 bytes in 0 blocks.
   ==21322== malloc/free: 11,241,958 allocs, 11,241,958 frees, 440,381,362 bytes allocated.
   ==21322== For counts of detected errors, rerun with: -v
   ==21322== All heap blocks were freed -- no leaks are possible.

---

 btt/Makefile      |    4 +--
 btt/args.c        |   51 ++++++++++++++++++++++++------------
 btt/bno_dump.c    |    8 ++++++
 btt/bt_timeline.c |   48 +++++++++++++++++++++-------------
 btt/devmap.c      |   10 +++++++
 btt/devs.c        |   45 +++++++++++++++++++++++++++++++-
 btt/globals.h     |   14 +++++++---
 btt/inlines.h     |   75 +++++++++++++++++++++++++++++++++--------------------
 btt/misc.c        |   35 ++++++++++++++++++++-----
 btt/output.c      |    9 +++---
 btt/proc.c        |   32 +++++++++++++++++++----
 btt/seek.c        |   22 ++++++++++++++++
 12 files changed, 266 insertions(+), 87 deletions(-)
btt/Makefile +2 -2
index ba9e86f..fcb09a4 100644
@@ -3,8 +3,8 @@ CC	= gcc
 ECFLAGS	= 
 # ECFLAGS	= -DCOUNT_IOS
 
-CFLAGS	= -Wall -O2 -W -g $(ECFLAGS)
-# CFLAGS	= -Wall -g -W -UDO_INLINE -DDEBUG $(ECFLAGS)
+# CFLAGS	= -Wall -O2 -W -g $(ECFLAGS)
+CFLAGS	= -Wall -g -W -UDO_INLINE -DDEBUG $(ECFLAGS)
 
 ALL_CFLAGS = $(CFLAGS) -I.. -D_GNU_SOURCE -D_LARGEFILE_SOURCE \
 		-D_FILE_OFFSET_BITS=64
btt/args.c +34 -17

73 lines not on this page · raw message

btt/bno_dump.c +8 -0
index c990171..d1bc6ab 100644
@@ -58,6 +58,14 @@ void *bno_dump_init(__u32 device)
 	return bdp;
 }
 
+void bno_dump_exit(void *param)
+{
+	/*
+	 * Associated files will be auto-cleaned by bno_dump_clean
+	 */
+	free(param);
+}
+
 static inline void bno_dump_write(FILE *fp, struct io *iop)
 {
 	fprintf(fp, "%15.9lf %lld %lld\n",
btt/bt_timeline.c +30 -18

78 lines not on this page · raw message

btt/devmap.c +10 -0
index 2b6366c..a74f224 100644
@@ -23,6 +23,16 @@
 
 struct devmap *all_devmaps = NULL;
 
+void dev_map_exit(void)
+{
+	struct devmap *dmp;
+
+	while ((dmp = all_devmaps) != NULL) {
+		all_devmaps = dmp->next;
+		free(dmp);
+	}
+}
+
 void dev_map_add(struct devmap *dmp)
 {
 	struct devmap *this = malloc(sizeof(struct devmap));
btt/devs.c +44 -1

63 lines not on this page · raw message

btt/globals.h +10 -4
index 69c9e9d..1b54cb0 100644
@@ -84,7 +84,7 @@ enum iop_type {
 struct file_info {
 	struct file_info *next;
 	FILE *ofp;
-	char oname[1];
+	char *oname;
 };
 
 struct mode {
@@ -124,7 +124,6 @@ struct range_info {
 struct region_info {
 	struct list_head qranges;
 	struct list_head cranges;
-	struct range_info *qr_cur, *cr_cur;
 };
 
 struct p_info {
@@ -132,7 +131,7 @@ struct p_info {
 	struct avgs_info avgs;
 	__u64 last_q;
 	__u32 pid;
-	char name[1];
+	char *name;
 };
 
 struct devmap {
@@ -222,9 +221,10 @@ extern struct list_head cios;
 /* args.c */
 void handle_args(int argc, char *argv[]);
 
-/* dev_map.c */
+/* devmap.c */
 int dev_map_read(char *fname);
 struct devmap *dev_map_find(__u32 device);
+void dev_map_exit(void);
 
 /* devs.c */
 #if defined(DEBUG)
@@ -241,6 +241,7 @@ struct io *dip_find_sec(struct d_info *dip, enum iop_type type, __u64 sec);
 void dip_foreach_out(void (*func)(struct d_info *, void *), void *arg);
 void dip_plug(__u32 dev, double cur_time);
 void dip_unplug(__u32 dev, double cur_time, int is_timer);
+void dip_exit(void);
 
 /* dip_rb.c */
 int rb_insert(struct rb_root *root, struct io *iop);
@@ -269,6 +270,8 @@ void latency_q2c(struct d_info *dip, __u64 tstamp, __u64 latency);
 int in_devices(struct blk_io_trace *t);
 void add_file(struct file_info **fipp, FILE *fp, char *oname);
 void clean_files(struct file_info **fipp);
+void add_buf(void *buf);
+void clean_bufs(void);
 void dbg_ping(void);
 
 /* mmap.c */
@@ -286,14 +289,17 @@ void add_process(__u32 pid, char *name);
 struct p_info *find_process(__u32 pid, char *name);
 void pip_update_q(struct io *iop);
 void pip_foreach_out(void (*f)(struct p_info *, void *), void *arg);
+void pip_exit(void);
 
 /* bno_dump.c */
 void *bno_dump_init(__u32 device);
+void bno_dump_exit(void *param);
 void bno_dump_add(void *handle, struct io *iop);
 void bno_dump_clean(void);
 
 /* seek.c */
 void *seeki_init(__u32 device);
+void seeki_exit(void *param);
 void seek_clean(void);
 void seeki_add(void *handle, struct io *iop);
 double seeki_mean(void *handle);
btt/inlines.h +46 -29

110 lines not on this page · raw message

btt/misc.c +29 -6
index 022cef5..24ade81 100644
@@ -53,15 +53,12 @@ int in_devices(struct blk_io_trace *t)
 
 void add_file(struct file_info **fipp, FILE *fp, char *oname)
 {
-	struct file_info *fip;
-	
-	fip = malloc(sizeof(struct file_info) + strlen(oname) + 1);
+	struct file_info *fip = malloc(sizeof(*fip));
 
+	fip->ofp = fp;
+	fip->oname = oname;
 	fip->next = *fipp;
 	*fipp = fip;
-
-	fip->ofp = fp;
-	strcpy(fip->oname, oname);
 }
 
 void clean_files(struct file_info **fipp)
@@ -75,8 +72,34 @@ void clean_files(struct file_info **fipp)
 		fclose(fip->ofp);
 		if (!stat(fip->oname, &buf) && (buf.st_size == 0))
 			unlink(fip->oname);
+
+		free(fip->oname);
 		free(fip);
 	}
 }
 
+struct buf_info {
+	struct buf_info *next;
+	void *buf;
+} *all_bufs;
+void add_buf(void *buf)
+{
+	struct buf_info *bip = malloc(sizeof(*bip));
+
+	bip->buf = buf;
+	bip->next = all_bufs;
+	all_bufs = bip;
+}
+
+void clean_bufs(void)
+{
+	struct buf_info *bip;
+
+	while ((bip = all_bufs) != NULL) {
+		all_bufs = bip->next;
+		free(bip->buf);
+		free(bip);
+	}
+}
+	
 void dbg_ping(void) {}
btt/output.c +4 -5
index 6851848..18d1d53 100644
@@ -321,6 +321,7 @@ void output_seek_mode_info(FILE *ofp, struct o_seek_info *sip)
 		free(p);
 	}
 }
+
 void add_seek_mode_info(struct o_seek_info *sip, struct mode *mp)
 {
 	int i;
@@ -342,6 +343,8 @@ void add_seek_mode_info(struct o_seek_info *sip, struct mode *mp)
 			sip->head = new;
 			new->mode = lp[i];
 			new->nseeks = mp->most_seeks;
+
+			add_buf(new);
 		}
 	}
 }
@@ -373,6 +376,7 @@ void __output_dip_seek_info(struct d_info *dip, void *arg)
 		seek_info.mean += (nseeks * mean);
 		seek_info.median += (nseeks * median);
 		add_seek_mode_info(&seek_info, &m);
+		free(m.modes);
 	}
 }
 
@@ -598,11 +602,6 @@ void __output_ranges(FILE *ofp, struct list_head *head_p, float base)
 int output_regions(FILE *ofp, char *header, struct region_info *reg, 
 			  float base)
 {
-	if (reg->qr_cur != NULL)
-		list_add_tail(&reg->qr_cur->head, &reg->qranges);
-	if (reg->cr_cur != NULL)
-		list_add_tail(&reg->cr_cur->head, &reg->cranges);
-
 	if (list_len(&reg->qranges) == 0 && list_len(&reg->cranges) == 0)
 		return 0;
 
btt/proc.c +27 -5
index 8e6c5ff..e24948f 100644
@@ -33,6 +33,24 @@ struct pn_info {
 
 struct rb_root root_pid, root_name;
 
+static void __destroy(struct rb_node *n, int free_name, int free_pip)
+{
+	if (n) {
+		struct pn_info *pnp = rb_entry(n, struct pn_info, rb_node);
+
+		__destroy(n->rb_left, free_name, free_pip);
+		__destroy(n->rb_right, free_name, free_pip);
+
+		if (free_name) free(pnp->u.name);
+		if (free_pip) {
+			free(pnp->pip->name);
+			region_exit(&pnp->pip->regions);
+			free(pnp->pip);
+		}
+		free(pnp);
+	}
+}
+
 struct p_info * __find_process_pid(__u32 pid)
 {
 	struct pn_info *this;
@@ -151,13 +169,11 @@ void add_process(__u32 pid, char *name)
 	struct p_info *pip = find_process(pid, name);
 
 	if (pip == NULL) {
-		size_t len = sizeof(struct p_info) + strlen(name) + 1;
-
-		pip = memset(malloc(len), 0, len);
+		pip = memset(malloc(sizeof(*pip)), 0, sizeof(*pip));
 		pip->pid = pid;
-		init_region(&pip->regions);
+		region_init(&pip->regions);
 		pip->last_q = (__u64)-1;
-		strcpy(pip->name, name);
+		pip->name = strdup(name);
 
 		insert(pip);
 	}
@@ -204,3 +220,9 @@ void pip_foreach_out(void (*f)(struct p_info *, void *), void *arg)
 		}
 	}
 }
+
+void pip_exit(void)
+{
+	__destroy(root_pid.rb_node, 0, 0);
+	__destroy(root_name.rb_node, 1, 1);
+}
btt/seek.c +22 -0
index f5f3851..e23f6bb 100644
@@ -84,6 +84,17 @@ static void __insert(struct rb_root *root, long long sectors)
 	rb_insert_color(&sbp->rb_node, root);
 }
 
+static void __destroy(struct rb_node *n)
+{
+	if (n) {
+		struct seek_bkt *sbp = rb_entry(n, struct seek_bkt, rb_node);
+
+		__destroy(n->rb_left);
+		__destroy(n->rb_right);
+		free(sbp);
+	}
+}
+
 void seek_clean(void)
 {
 	clean_files(&seek_files);
@@ -122,6 +133,17 @@ void *seeki_init(__u32 device)
 	return sip;
 }
 
+void seeki_exit(void *param)
+{
+	struct seeki *sip = param;
+
+	/*
+	 * Associated files are cleaned up by seek_clean
+	 */
+	__destroy(sip->root.rb_node);
+	free(sip);
+}
+
 void seeki_add(void *handle, struct io *iop)
 {
 	struct seeki *sip = handle;
#1 lore raw
Quoted 32 lines · Alan D. Brunelle

Ensure that all allocated memory was released.

Lots of minor clean up to help ensure all allocated memory is freed prior to exit. (Also closed a few files that were left open.)

# valgrind --leak-check=yes --show-reachable=yes btt -i bp.bin -o btt
-v
=21322= Memcheck, a memory error detector.
=21322= Copyright (C) 2002-2006, and GNU GPL'd, by Julian Seward et
al.
=21322= Using LibVEX rev 1658, a library for dynamic binary
translation.
=21322= Copyright (C) 2004-2006, and GNU GPL'd, by OpenWorks LLP.
=21322= Using valgrind-3.2.1-Debian, a dynamic binary
instrumentation framework.
=21322= Copyright (C) 2000-2006, and GNU GPL'd, by Julian Seward et
al.
=21322= For more details, rerun with: -v
=21322= 
Sending range data to btt.dat
Sending stats data to btt.avg
   4581291 traces @ 10.7 Ktps in 426.578804
seconds                                     tree = |0|
   =21322= 
   =21322= ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 8
from 1)
   =21322= malloc/free: in use at exit: 0 bytes in 0 blocks.
   =21322= malloc/free: 11,241,958 allocs, 11,241,958 frees,
440,381,362 bytes allocated.
   =21322= For counts of detected errors, rerun with: -v
   =21322= All heap blocks were freed -- no leaks are possible.
Quoted 2 lines

---

cool. really great to see this.

#3 lore raw
In reply to Alan D. Brunelle
Quoted 5 lines · Alan D. Brunelle

This has a "small" bug in it - for some reason the .dat file containing "ranges" now contains every IO.

Looking into it...

Alan

From: Alan D. Brunelle <[email protected]>

Fix range check: use proper data type for comparison

---

 btt/inlines.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
btt/inlines.h +1 -1
index 3c8deac..edb2182 100644
@@ -52,7 +52,7 @@ static inline void update_range(struct list_head *head_p, __u64 time)
 		if (time < rip->end)
 			return;
 
-		if ((time - rip->end) < range_delta) {
+		if (BIT_TIME(time - rip->end) < range_delta) {
 			rip->end = time;
 			return;
 		}