Re: [Evolution-hackers] Possible memory leak

[Philip Van Hoof <spam pvanhoof be>]

On Mon, 2008-01-14 at 01:47 +0100, Philip Van Hoof wrote:

> I have this memory analysis tool that I'm willing to believe that tells
> me this line in camel-folder.c causes 381 times a leaked allocation of
> in total 5.95 kb. (I opened one folder of 1000 items). It seems to be
> the g_slice_alloc (which does a malloc since I disabled gslice) of the
> g_hash_node_new.

I found it, this was my own mistake in camel-lite's adaptations.

I have found another leak in camel-operation.c:

The cc->status_stack = g_slist_prepend(cc->status_stack, s); in
camel_operation_start seems to be leaked each time it's called.

I have 23 calls leaking 184 bytes. Various callers of
camel_operation_start (all of them, it seems) seem to create this leak.

My version of camel-operation.c adds two fields (a total and a current
position in stead of a percentage, because my requirements where to give
accurate to-the-byte progress info, not rounded percentages). Those
changes are closely reviewed once more and don't seem to be causing this
leak.

ps. I attached the differences that I have. I basically replaced "pc"
with a "sofar" and a "oftotal".


-- 
Philip Van Hoof, freelance software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://pvanhoof.be/blog
http://codeminded.be

--- camel-operation.c	2007-12-23 18:12:33.000000000 +0100
+++ /home/pvanhoof/repos/tinymail/trunk/libtinymail-camel/camel-lite/camel/camel-operation.c	2007-11-15 10:42:00.000000000 +0100
@@ -46,7 +46,7 @@
 struct _status_stack {
 	guint32 flags;
 	char *msg;
-	int pc;				/* last pc reported */
+	int sofar, oftotal;
 	unsigned int stamp;		/* last stamp reported */
 };
 
@@ -514,12 +514,14 @@
 	s = g_malloc0(sizeof(*s));
 	s->msg = msg;
 	s->flags = 0;
+	s->sofar = 0;
+	s->oftotal = 100;
 	cc->lastreport = s;
 	cc->status_stack = g_slist_prepend(cc->status_stack, s);
 
 	UNLOCK();
 
-	cc->status(cc, msg, CAMEL_OPERATION_START, cc->status_data);
+	cc->status(cc, msg, CAMEL_OPERATION_START, 100, cc->status_data);
 
 	d(printf("start '%s'\n", msg, pc));
 }
@@ -578,17 +580,17 @@
 /**
  * camel_operation_progress:
  * @cc: Operation to report to.
- * @pc: Percent complete, 0 to 100.
+ * @sofar: Amount complete
+ * @oftotal: Max amount (or, amount when completed)
  *
  * Report progress on the current operation.  If @cc is NULL, then the
- * currently registered operation is used.  @pc reports the current
- * percentage of completion, which should be in the range of 0 to 100.
+ * currently registered operation is used.  @
  *
  * If the total percentage is not know, then use
  * camel_operation_progress_count().
  **/
 void
-camel_operation_progress (CamelOperation *cc, int pc)
+camel_operation_progress (CamelOperation *cc, int sofar, int oftotal)
 {
 	unsigned int now;
 	struct _status_stack *s;
@@ -608,7 +610,8 @@
 	}
 
 	s = cc->status_stack->data;
-	s->pc = pc;
+	s->sofar = sofar;
+	s->oftotal = oftotal;
 
 	/* Transient messages dont start updating till 4 seconds after
 	   they started, then they update every second */
@@ -632,7 +635,7 @@
 	UNLOCK();
 
 	if (cc) {
-		cc->status(cc, msg, pc, cc->status_data);
+		cc->status(cc, msg, sofar, oftotal, cc->status_data);
 		g_free(msg);
 	}
 }
@@ -646,7 +649,7 @@
 void
 camel_operation_progress_count (CamelOperation *cc, int sofar)
 {
-	camel_operation_progress(cc, sofar);
+	camel_operation_progress(cc, sofar, 100);
 }
 
 /**
@@ -664,7 +667,8 @@
 	struct _status_stack *s, *p;
 	unsigned int now;
 	char *msg = NULL;
-	int pc = 0;
+	int sofar = 0;
+	int oftotal = 100;
 
 	if (cc == NULL)
 		cc = co_getcc();
@@ -693,13 +697,15 @@
 				if (p->flags & CAMEL_OPERATION_TRANSIENT) {
 					if (p->stamp + CAMEL_OPERATION_TRANSIENT_DELAY < now) {
 						msg = g_strdup(p->msg);
-						pc = p->pc;
+						sofar = p->sofar;
+						oftotal = p->oftotal;
 						cc->lastreport = p;
 						break;
 					}
 				} else {
 					msg = g_strdup(p->msg);
-					pc = p->pc;
+					sofar = p->sofar;
+					oftotal = p->oftotal;
 					cc->lastreport = p;
 					break;
 				}
@@ -709,7 +715,8 @@
 		g_free(s->msg);
 	} else {
 		msg = s->msg;
-		pc = CAMEL_OPERATION_END;
+		sofar = CAMEL_OPERATION_END;
+		oftotal = 100;
 		cc->lastreport = s;
 	}
 	g_free(s);
@@ -718,7 +725,7 @@
 	UNLOCK();
 
 	if (msg) {
-		cc->status(cc, msg, pc, cc->status_data);
+		cc->status(cc, msg, sofar, oftotal, cc->status_data);
 		g_free(msg);
 	}
 }