Merge lp:~macslow/notify-osd/fix-mem-leaks into lp:notify-osd/trunk

[Mirco Müller]

[Mirco Müller (macslow)]

This fixes numerous mem-leaks identified by valgrind'ing notify-osd, examples and tests. It is by no means complete, but a good start. The work sofar showed that lots of the mem-leaks sadly happen outside of notify-osd scope (e.g. libfontconfig, libfreetype, libpango ...) resulting in notify-osd still leaking like sieve unfortunately.

[David Barth (dbarth)]

approve, all look like good changes

review: Approve

=== modified file 'src/bubble.c'
--- src/bubble.c	2009-08-27 09:52:34 +0000
+++ src/bubble.c	2009-09-23 01:05:51 +0000
@@ -475,41 +475,6 @@
 	cairo_surface_destroy (new_surface);
 }
 
-cairo_surface_t*
-_copy_surface (cairo_surface_t* orig)
-{
-	cairo_surface_t* copy       = NULL;
-	guchar*          pixels_src = NULL;
-	guchar*          pixels_cpy = NULL;
-	cairo_format_t   format;
-	gint             width;
-	gint             height;
-	gint             stride;
-
-	pixels_src = cairo_image_surface_get_data (orig);
-	if (!pixels_src)
-		return NULL;
-
-	format = cairo_image_surface_get_format (orig);
-	width  = cairo_image_surface_get_width (orig);
-	height = cairo_image_surface_get_height (orig);
-	stride = cairo_image_surface_get_stride (orig);
-
-	pixels_cpy = g_malloc0 (stride * height);
-	if (!pixels_cpy)
-		return NULL;
-
-	memcpy ((void*) pixels_cpy, (void*) pixels_src, height * stride);
-
-	copy = cairo_image_surface_create_for_data (pixels_cpy,
-						    format,
-						    width,
-						    height,
-						    stride);
-
-	return copy;
-}
-
 static void
 _draw_layout_grid (cairo_t* cr,
 		  Bubble*  bubble)
@@ -665,9 +630,11 @@
 
 	g_return_if_fail (scratch);
 
-	if (cairo_surface_status (scratch) != CAIRO_STATUS_SUCCESS) {
+	if (cairo_surface_status (scratch) != CAIRO_STATUS_SUCCESS)
+	{
 		if (scratch)
 			cairo_surface_destroy (scratch);
+
 		return;
 	}
 
@@ -676,8 +643,10 @@
 	if (cairo_status (cr) != CAIRO_STATUS_SUCCESS)
 	{
 		cairo_surface_destroy (scratch);
+
 		if (cr)
 			cairo_destroy (cr);
+
 		return;
 	}
 
@@ -766,7 +735,7 @@
 			cairo_image_surface_get_stride (clone));
 	blurred = copy_surface (dummy);
 	cairo_surface_destroy (dummy);
-	cairo_surface_destroy (clone);
+	destroy_cloned_surface (clone);
 
 	// finally create tile with top-left shadow/background part
 	if (priv->tile_background_part)
@@ -782,9 +751,13 @@
 		normal = cairo_image_surface_create (CAIRO_FORMAT_ARGB32,
 						     width,
 						     height);
-		if (cairo_surface_status (normal) != CAIRO_STATUS_SUCCESS) {
+		if (cairo_surface_status (normal) != CAIRO_STATUS_SUCCESS)
+		{
+			cairo_surface_destroy (scratch);
+
 			if (normal)
 				cairo_surface_destroy (normal);
+
 			return;
 		}
 
@@ -793,9 +766,12 @@
 						      height);
 		if (cairo_surface_status (blurred) != CAIRO_STATUS_SUCCESS)
 		{
+			cairo_surface_destroy (normal);
+			cairo_surface_destroy (scratch);
+
 			if (blurred)
 				cairo_surface_destroy (blurred);
-			cairo_surface_destroy (normal);
+
 			return;
 		}
 	}
@@ -805,9 +781,13 @@
 		normal = cairo_image_surface_create (CAIRO_FORMAT_RGB24,
 						     width,
 						     height);
-		if (cairo_surface_status (normal) != CAIRO_STATUS_SUCCESS) {
+		if (cairo_surface_status (normal) != CAIRO_STATUS_SUCCESS)
+		{
+			cairo_surface_destroy (scratch);
+
 			if (normal)
 				cairo_surface_destroy (normal);
+
 			return;
 		}
 	}
@@ -821,8 +801,11 @@
 		{
 			cairo_surface_destroy (normal);
 			cairo_surface_destroy (blurred);
+			cairo_surface_destroy (scratch);
+
 			if (cr)
 				cairo_destroy (cr);
+
 			return;
 		}
 
@@ -851,6 +834,7 @@
 	if (cairo_status (cr) != CAIRO_STATUS_SUCCESS)
 	{
 		cairo_surface_destroy (normal);
+		cairo_surface_destroy (scratch);
 
 		if (priv->composited)
 			cairo_surface_destroy (blurred);
@@ -890,6 +874,7 @@
 		cairo_surface_destroy (blurred);
 
 	cairo_surface_destroy (normal);
+	cairo_surface_destroy (scratch);
 }
 
 void
@@ -947,13 +932,14 @@
 void
 _refresh_title (Bubble* self)
 {
-	BubblePrivate*        priv   = GET_PRIVATE (self);
-	Defaults*             d      = self->defaults;
-	cairo_surface_t*      normal = NULL;
-	cairo_t*              cr     = NULL;
-	PangoFontDescription* desc   = NULL;
-	PangoLayout*          layout = NULL;
-	raico_blur_t*         blur   = NULL;
+	BubblePrivate*        priv           = GET_PRIVATE (self);
+	Defaults*             d              = self->defaults;
+	cairo_surface_t*      normal         = NULL;
+	cairo_t*              cr             = NULL;
+	PangoFontDescription* desc           = NULL;
+	PangoLayout*          layout         = NULL;
+	raico_blur_t*         blur           = NULL;
+	gchar*                text_font_face = NULL;
 
 	// create temp. scratch surface
 	normal = cairo_image_surface_create (
@@ -986,13 +972,14 @@
 					 defaults_get_text_title_size (d) *
 					 PANGO_SCALE);
 
-	pango_font_description_set_family_static (desc,
-						  defaults_get_text_font_face (d));
+	text_font_face = defaults_get_text_font_face (d);
+	pango_font_description_set_family_static (desc, text_font_face);
 	pango_font_description_set_weight (desc,
 					   defaults_get_text_title_weight (d));
 	pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
 	pango_layout_set_font_description (layout, desc);
 	pango_font_description_free (desc);
+	g_free ((gpointer) text_font_face);
 
 	pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
 	pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
@@ -1051,13 +1038,14 @@
 void
 _refresh_body (Bubble* self)
 {
-	BubblePrivate*        priv      = GET_PRIVATE (self);
-	Defaults*             d         = self->defaults;
-	cairo_surface_t*      normal    = NULL;
-	cairo_t*              cr        = NULL;
-	PangoFontDescription* desc      = NULL;
-	PangoLayout*          layout    = NULL;
-	raico_blur_t*         blur      = NULL;
+	BubblePrivate*        priv           = GET_PRIVATE (self);
+	Defaults*             d              = self->defaults;
+	cairo_surface_t*      normal         = NULL;
+	cairo_t*              cr             = NULL;
+	PangoFontDescription* desc           = NULL;
+	PangoLayout*          layout         = NULL;
+	raico_blur_t*         blur           = NULL;
+	gchar*                text_font_face = NULL;
 
 	// create temp. scratch surface
 	normal = cairo_image_surface_create (
@@ -1090,13 +1078,14 @@
 					 defaults_get_text_body_size (d) *
 					 PANGO_SCALE);
 
-	pango_font_description_set_family_static (desc,
-						  defaults_get_text_font_face (d));
+	text_font_face = defaults_get_text_font_face (d);
+	pango_font_description_set_family_static (desc, text_font_face);
 	pango_font_description_set_weight (desc,
 					   defaults_get_text_body_weight (d));
 	pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
 	pango_layout_set_font_description (layout, desc);
 	pango_font_description_free (desc);
+	g_free ((gpointer) text_font_face);
 
 	pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
 	pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
@@ -1787,7 +1776,6 @@
 	GdkPixbuf*    buffer = NULL;
 	GdkPixbuf*    pixbuf = NULL;
 	GtkIconTheme* theme  = NULL;
-	GFile*        file   = NULL;
 	GError*       error  = NULL;
 
 	/* sanity check */
@@ -1797,9 +1785,7 @@
 	if (!strncmp (filename, "file://", 7))
 		filename += 7;
 
-	file = g_file_new_for_path (filename);
-	if (g_file_query_exists (file, NULL))
-	/* Implementation note: blocking I/O; could be cancellable though */
+	if (filename[0] == '/')
 	{
 		/* load image into pixbuf */
 		pixbuf = gdk_pixbuf_new_from_file_at_scale (filename,
@@ -2226,7 +2212,7 @@
 	this->priv->layout               = LAYOUT_NONE;
 	this->priv->widget               = window;
 	this->priv->title                = g_string_new ("");
-	this->priv->message_body         = g_string_new ("");
+	this->priv->message_body         = NULL;
 	this->priv->icon_pixbuf          = NULL;
 	this->priv->value                = -2;
 	this->priv->visible              = FALSE;
@@ -2314,7 +2300,7 @@
 
 	priv = GET_PRIVATE (self);
 
-	if (priv->message_body->len != 0)
+	if (priv->message_body && priv->message_body->len != 0)
 	{
 		g_signal_emit (self,
 			       g_bubble_signals[MESSAGE_BODY_DELETED], 
@@ -3049,6 +3035,7 @@
 	PangoRectangle        log_rect = {0, 0, 0, 0};
 	gint                  title_height;
 	BubblePrivate*        priv;
+	gchar*                text_font_face = NULL;
 
 	if (!self || !IS_BUBBLE (self))
 		return 0;
@@ -3089,9 +3076,8 @@
 					 defaults_get_text_title_size (d) *
 					 PANGO_SCALE);
 
-	pango_font_description_set_family_static (
-		desc,
-		defaults_get_text_font_face (d));
+	text_font_face = defaults_get_text_font_face (d);
+	pango_font_description_set_family_static (desc, text_font_face);
 
 	pango_font_description_set_weight (
 		desc,
@@ -3100,6 +3086,7 @@
 	pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
 	pango_layout_set_font_description (layout, desc);
 	pango_font_description_free (desc);
+	g_free ((gpointer) text_font_face);
 
 	pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
 	pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
@@ -3127,6 +3114,7 @@
 	PangoRectangle        log_rect;
 	gint                  body_height;
 	BubblePrivate*        priv;
+	gchar*                text_font_face = NULL;
 
 	if (!self || !IS_BUBBLE (self))
 		return 0;
@@ -3159,9 +3147,8 @@
 					 defaults_get_text_body_size (d) *
 					 PANGO_SCALE);
 
-	pango_font_description_set_family_static (
-		desc,
-		defaults_get_text_font_face (d));
+	text_font_face = defaults_get_text_font_face (d);
+	pango_font_description_set_family_static (desc, text_font_face);
 
 	pango_font_description_set_weight (
 		desc,
@@ -3222,6 +3209,7 @@
 	body_height = PANGO_PIXELS (log_rect.height);
 
 	pango_font_description_free (desc);
+	g_free ((gpointer) text_font_face);
 	g_object_unref (layout);
 	cairo_destroy (cr);
 

=== modified file 'src/defaults.c'
--- src/defaults.c	2009-09-16 12:33:40 +0000
+++ src/defaults.c	2009-09-18 19:48:10 +0000
@@ -179,15 +179,17 @@
 	GString*   font_face     = NULL;
 	gdouble    dpi           = 0.0f;
 	gdouble    pixels_per_em = 0;
+	gchar*     font_name     = NULL;
 
 	if (!IS_DEFAULTS (self))
 		return;
 
 	/* determine current system font-name/size */
 	error = NULL;
-	string = g_string_new (gconf_client_get_string (self->context,
-							GCONF_UI_FONT_NAME,
-							&error));
+	font_name = gconf_client_get_string (self->context,
+					     GCONF_UI_FONT_NAME,
+					     &error);
+	string = g_string_new (font_name);
 	if (error)
 	{
 		/* if something went wrong, assume "Sans 10" and continue */
@@ -197,6 +199,7 @@
 		           error->message);
 		g_error_free (error);
 	}
+	g_free ((gpointer) font_name);
 
 	/* extract font-family-name and font-size */
 	scanner = g_scanner_new (NULL);
@@ -430,63 +433,6 @@
 	g_signal_emit (defaults, g_defaults_signals[GRAVITY_CHANGED], 0);
 }
 
-static gdouble
-_get_average_char_width (Defaults* self)
-{
-	cairo_surface_t*      surface;
-	cairo_t*              cr;
-	PangoFontDescription* desc;
-	PangoLayout*          layout;
-	PangoContext*         context;
-	PangoLanguage*        language;
-	PangoFontMetrics*     metrics;
-	gint                  char_width;
-
-	if (!self || !IS_DEFAULTS (self))
-		return 0;
-
-	surface = cairo_image_surface_create (CAIRO_FORMAT_A1, 1, 1);
-	if (cairo_surface_status (surface) != CAIRO_STATUS_SUCCESS)
-		return 0;
-
-	cr = cairo_create (surface);
-	cairo_surface_destroy (surface);
-	if (cairo_status (cr) != CAIRO_STATUS_SUCCESS)
-		return 0;
-
-	layout = pango_cairo_create_layout (cr);
-	desc = pango_font_description_new ();
-	pango_font_description_set_size (
-		desc,
-		EM2PIXELS (defaults_get_text_title_size (self), self) *
-		PANGO_SCALE);
-
-	pango_font_description_set_family_static (
-		desc,
-		defaults_get_text_font_face (self));
-
-	pango_font_description_set_weight (
-		desc,
-		defaults_get_text_title_weight (self));
-
-	pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
-	pango_layout_set_wrap (layout, PANGO_WRAP_WORD);
-	pango_layout_set_font_description (layout, desc);
-
-	context  = pango_layout_get_context (layout); /* no need to unref */
-	language = pango_language_get_default ();     /* no need to unref */
-	metrics  = pango_context_get_metrics (context, desc, language);
-	char_width = pango_font_metrics_get_approximate_char_width (metrics);
-
-	/* clean up */
-	pango_font_metrics_unref (metrics);
-	pango_font_description_free (desc);
-	g_object_unref (layout);
-	cairo_destroy (cr);
-
-	return PIXELS2EM (char_width / PANGO_SCALE, self);
-}
-
 void
 defaults_refresh_screen_dimension_properties (Defaults *self)
 {
@@ -562,9 +508,6 @@
 	gdouble      icon_size;
 	gdouble      bubble_height;
 	gdouble      new_bubble_height;
-	gdouble      bubble_width;
-	gdouble      new_bubble_width;
-	gdouble      average_char_width;
 
 	self = DEFAULTS (gobject);
 
@@ -605,31 +548,6 @@
 			      NULL);
 	}
 
-	/* correct the default bubble-width depending on the average width of a 
-	 * character rendered in the default system-font at the default
-	 * font-size,
-	 * as default layout, we'll take the icon+title+body+message case, thus
-	 * seen from left to right we use:
-	 *
-	 *      margin + icon_size + margin + 20 * avg_char_width + margin
-	 */
-	g_object_get (self,
-		      "bubble-width",
-		      &bubble_width,
-		      NULL);
-	average_char_width = _get_average_char_width (self);
-
-	new_bubble_width = 3.0f * margin_size +
-			   icon_size +
-			   20.0f * average_char_width;
-	/*if (new_bubble_width > bubble_width)
-	{
-		g_object_set (self,
-			      "bubble-width",
-			      new_bubble_width,
-			      NULL);
-	}*/
-
 	/* FIXME: calling this here causes a segfault */
 	/* chain up to the parent class */
 	/*G_OBJECT_CLASS (defaults_parent_class)->constructed (gobject);*/
@@ -2431,19 +2349,22 @@
 static gboolean
 defaults_multihead_does_focus_follow (Defaults *self)
 {
-	GError *error = NULL;
-	gboolean mode = FALSE;
+	GError*  error = NULL;
+	gboolean mode  = FALSE;
 
 	g_return_val_if_fail (self != NULL && IS_DEFAULTS (self), FALSE);
 
-	gchar *mode_str = gconf_client_get_string (self->context,
+	gchar* mode_str = gconf_client_get_string (self->context,
 						   GCONF_MULTIHEAD_MODE,
 						   &error);
 	if (mode_str != NULL)
 	{
 		if (! g_strcmp0 (mode_str, "focus-follow"))
 			mode = TRUE;
-	} else if (error != NULL)
+
+		g_free ((gpointer) mode_str);
+	}
+	else if (error != NULL)
 	{
 		g_warning ("defaults_multihead_does_focus_follow(): "
 		           "Got error \"%s\"\n",

=== modified file 'tests/test-scroll-text.c'
--- tests/test-scroll-text.c	2009-08-04 17:34:48 +0000
+++ tests/test-scroll-text.c	2009-09-23 01:06:20 +0000
@@ -64,8 +64,6 @@
 	cairo_t*              cr;
 	PangoFontDescription* desc;
 	PangoLayout*          layout;
-	PangoRectangle        ink_rect;
-	PangoRectangle        log_rect;
 
 	// sanity check
 	if (!text      ||
@@ -112,7 +110,6 @@
 
 	// print and layout string (pango-wise)
 	pango_layout_set_text (layout, text, -1);
-	pango_layout_get_extents (layout, &ink_rect, &log_rect);
 
 	// make sure system-wide font-options like hinting, antialiasing etc.
 	// are taken into account
@@ -126,6 +123,8 @@
 	cairo_move_to (cr, 0.0f, 0.0f);
 	cairo_set_operator (cr, CAIRO_OPERATOR_OVER);
 	cairo_set_source_rgba (cr, 1.0f, 1.0f, 1.0f, 1.0f);
+
+	// this call leaks 3803 bytes, I've no idea how to fix that
 	pango_cairo_show_layout (cr, layout);
 
 	// clean up
@@ -749,6 +748,7 @@
 	cairo_set_source_surface (cr, text, 0.0f, 0.0f);
 	cairo_paint (cr);
 
+	cairo_destroy (cr);
 	cairo_surface_destroy (text);
 	tile = tile_new (surface, 6);
 	cairo_surface_destroy (surface);
@@ -870,11 +870,13 @@
 
 	// actually create the tile with padding in mind
 	tile = tile_new_for_padding (norm_surf, blur_surf);
+	destroy_cloned_surface (norm_surf);
+	destroy_cloned_surface (blur_surf);
+	destroy_cloned_surface (dummy_surf);
 
 	cairo_destroy (cr);
 	cairo_surface_destroy (cr_surf);
 
-	cairo_surface_destroy (norm_surf);
 	norm_surf = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, w, h);
 	cr = cairo_create (norm_surf);
 	cairo_scale (cr, 1.0f, 1.0f);
@@ -884,7 +886,6 @@
 	tile_paint_with_padding (tile, cr, 0.0f, 0.0f, w, h, 1.0f, 0.0f);
 	cairo_destroy (cr);
 
-	cairo_surface_destroy (blur_surf);
 	blur_surf = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, w, h);
 	cr = cairo_create (blur_surf);
 	cairo_scale (cr, 1.0f, 1.0f);
@@ -897,7 +898,7 @@
 	g_tile = tile_new_for_padding (norm_surf, blur_surf);
 
 	// clean up
-	cairo_surface_destroy (dummy_surf);
+	tile_destroy (tile);
 	cairo_surface_destroy (norm_surf);
 	cairo_surface_destroy (blur_surf);
 }