[evolution-data-server/openismus-work] Fixed memory leaks and reduced code size/minor cleanup.

[Tristan Van Berkom <tvb src gnome org>]

commit f14f37a8bebdc2f5cf49aa1eeb636a4b6f6fba46
Author: Tristan Van Berkom <tristan van berkom gmail com>
Date:   Fri Jul 15 18:48:30 2011 -0400

    Fixed memory leaks and reduced code size/minor cleanup.
    
    Fixed places where I was leaking EContactPhotos given
    to e_contact_set() (which does not assume ownership).
    
    Also added 'load_vcard()' and 'load_contact()' static functions
    to use intead of manually manipulating the BDB in many places.

 addressbook/backends/file/e-book-backend-file.c |  233 +++++++++++------------
 1 files changed, 114 insertions(+), 119 deletions(-)
---
diff --git a/addressbook/backends/file/e-book-backend-file.c b/addressbook/backends/file/e-book-backend-file.c
index dd39e8c..e2a6118 100644
--- a/addressbook/backends/file/e-book-backend-file.c
+++ b/addressbook/backends/file/e-book-backend-file.c
@@ -178,6 +178,49 @@ create_contact (const gchar *uid, const gchar *vcard)
 	return contact;
 }
 
+static gchar *
+load_vcard (EBookBackendFile *bf,
+	    const gchar      *uid,
+	    GError          **error)
+{
+	DB     *db = bf->priv->file_db;
+	DBT     id_dbt, vcard_dbt;
+	gchar  *vcard;
+	gint    db_error;
+
+	/* Get the old contact from the db and compare the photo fields */
+	string_to_dbt (uid, &id_dbt);
+	memset (&vcard_dbt, 0, sizeof (vcard_dbt));
+	vcard_dbt.flags = DB_DBT_MALLOC;
+		
+	db_error = db->get (db, NULL, &id_dbt, &vcard_dbt, 0);
+
+	if (db_error == 0) {
+		vcard = vcard_dbt.data;
+	} else {
+		g_warning (G_STRLOC ": db->get failed with %s", db_strerror (db_error));
+		g_propagate_error (error, EDB_ERROR (CONTACT_NOT_FOUND));
+		return NULL;
+	}
+
+	return vcard;
+}
+
+static EContact *
+load_contact (EBookBackendFile *bf,
+	      const gchar      *uid,
+	      GError          **error)
+{
+	EContact *contact = NULL;
+	gchar    *vcard;
+
+	if ((vcard = load_vcard (bf, uid, error)) != NULL) {
+		contact = create_contact (uid, vcard);
+		g_free (vcard);
+	}
+
+	return contact;
+}
 
 static gchar *
 collect_uri_for_field (EContact         *old_contact,
@@ -236,30 +279,22 @@ maybe_delete_uri (EBookBackendFile *bf,
 static void
 maybe_delete_unused_uris (EBookBackendFile *bf,
 			  const gchar      *id,
+			  EContact         *old_contact,
 			  EContact         *new_contact)
 {
-	DB                *db = bf->priv->file_db;
-	DBT                id_dbt, vcard_dbt;
-	gchar             *uri_photo, *uri_logo, *vcard;
-	EContact          *old_contact;
-	gint               db_error;
-
-	/* Get the old contact from the db and compare the photo fields */
-	string_to_dbt (id, &id_dbt);
-	memset (&vcard_dbt, 0, sizeof (vcard_dbt));
-	vcard_dbt.flags = DB_DBT_MALLOC;
-		
-	db_error = db->get (db, NULL, &id_dbt, &vcard_dbt, 0);
+	GError            *error = NULL;
+	gchar             *uri_photo, *uri_logo;
 
-	if (db_error == 0) {
-		vcard = vcard_dbt.data;
-	} else {
-		g_warning (G_STRLOC ": db->get failed with %s", db_strerror (db_error));
-		return;
-	}
+	if (!old_contact) {
+		old_contact = load_contact (bf, id, &error);
 
-	old_contact = create_contact (id, vcard);
-	g_free (vcard);
+		if (!old_contact) {
+			g_warning (G_STRLOC ": failed to load contact %s", error->message);
+			g_error_free (error);
+			return;
+		}
+	} else
+		g_object_ref (old_contact);
 
 	/* If there is no new contact, collect all the uris to delete from old_contact
 	 *
@@ -269,11 +304,15 @@ maybe_delete_unused_uris (EBookBackendFile *bf,
 	uri_photo = collect_uri_for_field (old_contact, new_contact, E_CONTACT_PHOTO);
 	uri_logo  = collect_uri_for_field (old_contact, new_contact, E_CONTACT_LOGO);
 
-	if (uri_photo)
+	if (uri_photo) {
 		maybe_delete_uri (bf, uri_photo);
+		g_free (uri_photo);
+	}
 
-	if (uri_logo)
+	if (uri_logo) {
 		maybe_delete_uri (bf, uri_logo);
+		g_free (uri_logo);
+	}
 
 	g_object_unref (old_contact);
 }
@@ -429,6 +468,7 @@ is_backend_owned_uri (EBookBackendFile *bf,
 
 static PhotoModifiedStatus
 maybe_transform_vcard_field_for_photo (EBookBackendFile *bf,
+				       EContact         *old_contact,
 				       EContact         *contact,
 				       EContactField     field,
 				       GError          **error)
@@ -462,7 +502,7 @@ maybe_transform_vcard_field_for_photo (EBookBackendFile *bf,
 
 			status = STATUS_ERROR;
 		} else {
-			new_photo           = g_new (EContactPhoto, 1);
+			new_photo           = g_slice_new0 (EContactPhoto);
 			new_photo->type     = E_CONTACT_PHOTO_TYPE_URI;
 			new_photo->data.uri = uri;
 
@@ -471,21 +511,16 @@ maybe_transform_vcard_field_for_photo (EBookBackendFile *bf,
 			d(g_print ("Backend modified incomming binary blob to be %s:\n", uri));
 
 			status = STATUS_MODIFIED;
+
+			g_slice_free (EContactPhoto, new_photo);
 		}
 
 		g_free (uri);
 		g_free (new_photo_path);
 
 	} else { /* E_CONTACT_PHOTO_TYPE_URI */
-
-		DB                *db = bf->priv->file_db;
-		DBT                id_dbt, vcard_dbt;
-		gchar             *db_vcard = NULL;
-		gint               db_error;
 		const gchar       *uid;
-
- 		EContact          *old_contact;
-		EContactPhoto     *old_photo, *new_photo;
+		EContactPhoto     *old_photo = NULL, *new_photo;
 
 		/* First determine that the new contact uri points to our 'photos' directory,
 		 * if not then we do nothing
@@ -501,24 +536,8 @@ maybe_transform_vcard_field_for_photo (EBookBackendFile *bf,
 			return STATUS_ERROR;
 		}
 
-		string_to_dbt (uid, &id_dbt);
-		memset (&vcard_dbt, 0, sizeof (vcard_dbt));
-		vcard_dbt.flags = DB_DBT_MALLOC;
-		
-		db_error = db->get (db, NULL, &id_dbt, &vcard_dbt, 0);
-
-		if (db_error == 0) {
-			db_vcard = vcard_dbt.data;
-		} else {
-			g_warning (G_STRLOC ": db->get failed with %s", db_strerror (db_error));
-
-			db_error_to_gerror (db_error, error);
-			return STATUS_ERROR;
-		}
-
-		old_contact = create_contact (uid, db_vcard);
-		old_photo   = e_contact_get (old_contact, field);
-		g_free (db_vcard);
+		if (old_contact)
+			old_photo = e_contact_get (old_contact, field);
 
 		/* Unless we are receiving the same uri that we already have
 		 * stored in the BDB... */
@@ -552,12 +571,12 @@ maybe_transform_vcard_field_for_photo (EBookBackendFile *bf,
 				GError *local_err = NULL;
 				if (!remove_file (new_filename, &local_err)) {
 					g_warning ("Unable to cleanup photo uri: %s", local_err->message);
-					g_error_free (error);
+					g_error_free (local_err);
 				}
 				status = STATUS_ERROR;
 			} else {
 
-				new_photo           = g_new (EContactPhoto, 1);
+				new_photo           = g_slice_new0 (EContactPhoto);
 				new_photo->type     = E_CONTACT_PHOTO_TYPE_URI;
 				new_photo->data.uri = new_uri;
 
@@ -565,6 +584,7 @@ maybe_transform_vcard_field_for_photo (EBookBackendFile *bf,
 
 				d(g_print ("Backend modified incomming shared uri to be %s:\n", new_uri));
 
+				g_slice_free (EContactPhoto, new_photo);
 				status = STATUS_MODIFIED;
 			}
 			g_free (new_uri);
@@ -585,6 +605,7 @@ maybe_transform_vcard_field_for_photo (EBookBackendFile *bf,
  */
 static PhotoModifiedStatus
 maybe_transform_vcard_for_photo (EBookBackendFile *bf,
+				 EContact         *old_contact,
 				 EContact         *contact,
 				 gchar           **vcard_ret,
 				 GError          **error)
@@ -592,11 +613,13 @@ maybe_transform_vcard_for_photo (EBookBackendFile *bf,
 	PhotoModifiedStatus status;
 	gboolean            modified = FALSE;
 
-	status   = maybe_transform_vcard_field_for_photo (bf, contact, E_CONTACT_PHOTO, error);
+	status   = maybe_transform_vcard_field_for_photo (bf, old_contact, contact,
+							  E_CONTACT_PHOTO, error);
 	modified = (status == STATUS_MODIFIED);
 
 	if (status != STATUS_ERROR) {
-		status   = maybe_transform_vcard_field_for_photo (bf, contact, E_CONTACT_LOGO, error);
+		status   = maybe_transform_vcard_field_for_photo (bf, old_contact, contact,
+								  E_CONTACT_LOGO, error);
 		modified = modified || (status == STATUS_MODIFIED);
 	}
 
@@ -741,7 +764,7 @@ do_create(EBookBackendFile  *bf,
 
 		gchar *transformed_vcard = NULL;
 
-		status = maybe_transform_vcard_for_photo (bf, *contact, &transformed_vcard, perror);
+		status = maybe_transform_vcard_for_photo (bf, NULL, *contact, &transformed_vcard, perror);
 
 		if (status == STATUS_MODIFIED && transformed_vcard) {
 
@@ -802,7 +825,7 @@ e_book_backend_file_remove_contacts (EBookBackendSync *backend,
 		id = l->data;
 
 		/* First collect uris to delete */
-		maybe_delete_unused_uris (bf, id, NULL);
+		maybe_delete_unused_uris (bf, id, NULL, NULL);
 
 		/* Then go on to delete from the db */
 		string_to_dbt (id, &id_dbt);
@@ -847,6 +870,7 @@ e_book_backend_file_modify_contact (EBookBackendSync *backend,
 	const gchar    *id, *lookup_id;
 	gchar          *vcard_with_rev;
 	PhotoModifiedStatus status;
+	EContact       *old_contact;
 
 	*contact = e_contact_new_from_vcard (vcard);
 	id = e_contact_get_const (*contact, E_CONTACT_UID);
@@ -856,14 +880,23 @@ e_book_backend_file_modify_contact (EBookBackendSync *backend,
 		return;
 	}
 
+	old_contact = load_contact (bf, id, perror);
+	if (!old_contact) {
+		g_warning (G_STRLOC ": Failed to load contact %s", id);
+		return;
+	}
+
 	/* Transform incomming photo blobs to uris before storing this to the DB */
-	status = maybe_transform_vcard_for_photo (bf, *contact, NULL, perror);
-	if (status == STATUS_ERROR)
+	status = maybe_transform_vcard_for_photo (bf, old_contact, *contact, NULL, perror);
+	if (status == STATUS_ERROR) {
+		g_object_unref (old_contact);
 		return;
+	}
 
 	/* Delete old photo file uris if need be (this will compare the new contact
 	 * with the current copy in the BDB to extract the uris to delete) */
-	maybe_delete_unused_uris (bf, id, *contact);
+	maybe_delete_unused_uris (bf, id, old_contact, *contact);
+	g_object_unref (old_contact);
 
 	/* update the revision (modified time of contact) */
 	set_revision (*contact);
@@ -909,28 +942,12 @@ e_book_backend_file_get_contact (EBookBackendSync *backend,
 				 gchar **vcard,
 				 GError **perror)
 {
-	EBookBackendFile *bf;
-	DB             *db;
-	DBT             id_dbt, vcard_dbt;
-	gint             db_error = 0;
-
-	bf = E_BOOK_BACKEND_FILE (backend);
-	db = bf->priv->file_db;
-
-	string_to_dbt (id, &id_dbt);
-	memset (&vcard_dbt, 0, sizeof (vcard_dbt));
-	vcard_dbt.flags = DB_DBT_MALLOC;
+	EBookBackendFile *bf = E_BOOK_BACKEND_FILE (backend);
 
-	db_error = db->get (db, NULL, &id_dbt, &vcard_dbt, 0);
+	*vcard = load_vcard (bf, id, perror);
 
-	if (db_error == 0) {
-		*vcard = vcard_dbt.data;
-	} else {
-		g_warning (G_STRLOC ": db->get failed with %s", db_strerror (db_error));
+	if (!*vcard)
 		*vcard = g_strdup ("");
-
-		g_propagate_error (perror, EDB_ERROR (CONTACT_NOT_FOUND));
-	}
 }
 
 static void
@@ -965,18 +982,13 @@ e_book_backend_file_get_contact_list (EBookBackendSync *backend,
 
 		for (i = 0; i < ids->len; i++) {
 			gchar *id = g_ptr_array_index (ids, i);
-			string_to_dbt (id, &id_dbt);
-			memset (&vcard_dbt, 0, sizeof (vcard_dbt));
-			vcard_dbt.flags = DB_DBT_MALLOC;
+			gchar *vcard;
 
-			db_error = db->get (db, NULL, &id_dbt, &vcard_dbt, 0);
-			if (db_error == 0) {
-				contact_list = g_list_prepend (contact_list, vcard_dbt.data);
-			} else {
-				g_warning (G_STRLOC ": db->get failed with %s", db_strerror (db_error));
-				db_error_to_gerror (db_error, perror);
+			vcard = load_vcard (bf, id, perror);
+			if (!vcard)
 				break;
-			}
+
+			contact_list = g_list_prepend (contact_list, vcard);
 		}
 		g_ptr_array_free (ids, TRUE);
 	} else {
@@ -1255,6 +1267,7 @@ book_view_thread (gpointer data)
 
 		for (i = 0; i < ids->len; i++) {
 			gchar *id = g_ptr_array_index (ids, i);
+			gchar *vcard;
 
 			if (!e_flag_is_set (closure->running))
 				break;
@@ -1264,18 +1277,9 @@ book_view_thread (gpointer data)
 				continue;
 			}
 
-			string_to_dbt (id, &id_dbt);
-			memset (&vcard_dbt, 0, sizeof (vcard_dbt));
-			vcard_dbt.flags = DB_DBT_MALLOC;
-
-			db_error = db->get (db, NULL, &id_dbt, &vcard_dbt, 0);
-
-			if (db_error == 0) {
-				notify_update_vcard (book_view, uid_only, TRUE, id, vcard_dbt.data);
-			}
-			else {
-				g_warning (G_STRLOC ": db->get failed with %s", db_strerror (db_error));
-			}
+			vcard = load_vcard (bf, id, NULL);
+			if (vcard)
+				notify_update_vcard (book_view, uid_only, TRUE, id, vcard);
 		}
 
 		g_ptr_array_free (ids, TRUE);
@@ -1368,7 +1372,7 @@ e_book_backend_file_stop_book_view (EBookBackend  *backend,
 }
 
 typedef struct {
-	DB *db;
+	EBookBackendFile *bf;
 
 	GList *add_cards;
 	GList *add_ids;
@@ -1382,35 +1386,26 @@ static void
 e_book_backend_file_changes_foreach_key (const gchar *key, gpointer user_data)
 {
 	EBookBackendFileChangeContext *ctx = user_data;
-	DB      *db = ctx->db;
-	DBT     id_dbt, vcard_dbt;
-	gint     db_error = 0;
+	EBookBackendFile *bf = ctx->bf;
+	gchar            *vcard;
 
-	string_to_dbt (key, &id_dbt);
-	memset (&vcard_dbt, 0, sizeof (vcard_dbt));
-	vcard_dbt.flags = DB_DBT_MALLOC;
-
-	db_error = db->get (db, NULL, &id_dbt, &vcard_dbt, 0);
+	vcard = load_vcard (bf, key, NULL);
 
-	if (db_error != 0) {
+	if (vcard) {
 		EContact *contact;
-		gchar *id = id_dbt.data;
-		gchar *vcard_string;
+		gchar    *vcard_string;
 
 		contact = e_contact_new ();
-		e_contact_set (contact, E_CONTACT_UID, id);
+		e_contact_set (contact, E_CONTACT_UID, key);
 
 		vcard_string = e_vcard_to_string (E_VCARD (contact), EVC_FORMAT_VCARD_30);
 
-		ctx->del_ids = g_list_append (ctx->del_ids,
-					      g_strdup (id));
-		ctx->del_cards = g_list_append (ctx->del_cards,
-						vcard_string);
+		ctx->del_ids   = g_list_append (ctx->del_ids,   g_strdup (key));
+		ctx->del_cards = g_list_append (ctx->del_cards, vcard_string);
 
 		g_object_unref (contact);
+		g_free (vcard);
 	}
-
-	g_free (vcard_dbt.data);
 }
 
 static void
@@ -1437,7 +1432,7 @@ e_book_backend_file_get_changes (EBookBackendSync *backend,
 
 	memset (&ctx, 0, sizeof (ctx));
 
-	ctx.db = db;
+	ctx.bf = bf;
 
 	/* Find the changed ids */
 	filename = g_strdup_printf ("%s/%s" CHANGES_DB_SUFFIX, bf->priv->dirname, change_id);