[almanah] Bug 579242 – No error messages for bad keys when closing

[Philip Withnall <pwithnall src gnome org>]

commit 4fb9feb96fc1a20c8e0d259653b224d62e7ebbe5
Author: Philip Withnall <philip tecnocode co uk>
Date:   Sun May 3 15:42:40 2009 +0100

    Bug 579242 â?? No error messages for bad keys when closing
    
    Improved error reporting when closing the database, such that errors during
    encryption are now presented in error dialogues, rather than just being
    printed silently to the console.
---
 src/Makefile.am          |   17 +++++++++++++++
 src/almanah-marshal.list |    1 +
 src/main.c               |   35 ++++++++++++++++---------------
 src/storage-manager.c    |   50 +++++++++++++++++++++++----------------------
 4 files changed, 62 insertions(+), 41 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index a0be38e..92dac68 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -22,6 +22,7 @@ endif
 bin_PROGRAMS = almanah
 
 almanah_SOURCES = \
+	$(ALMANAH_MARSHAL_FILES)	\
 	definition-builtins.c	\
 	definition-builtins.h	\
 	event-factory-builtins.c	\
@@ -88,6 +89,19 @@ almanah_LDADD = \
 	$(ENCRYPTION_LIBS)	\
 	$(SPELL_CHECKING_LIBS)
 
+# Marshalling
+ALMANAH_MARSHAL_FILES = \
+	almanah-marshal.c	\
+	almanah-marshal.h
+
+GLIB_GENMARSHAL = `pkg-config --variable=glib_genmarshal glib-2.0`
+
+almanah-marshal.h: almanah-marshal.list Makefile
+	( $(GLIB_GENMARSHAL) --prefix=almanah_marshal $(srcdir)/almanah-marshal.list --header > almanah-marshal.h )
+almanah-marshal.c: almanah-marshal.h Makefile
+	( $(GLIB_GENMARSHAL) --prefix=almanah_marshal $(srcdir)/almanah-marshal.list --header --body > almanah-marshal.c )
+
+# Enums
 definition-builtins.h: stamp-definition-builtins.h
 	@true
 
@@ -137,6 +151,7 @@ event-factory-builtins.c: event-factory.h Makefile event-factory-builtins.h
 	&& rm -f xgen-gtbc
 
 CLEANFILES = \
+	$(ALMANAH_MARSHAL_FILES)	\
 	definition-builtins.h		\
 	definition-builtins.c		\
 	stamp-definition-builtins.h	\
@@ -144,4 +159,6 @@ CLEANFILES = \
 	event-factory-builtins.c		\
 	stamp-event-factory-builtins.h
 
+EXTRA_DIST = almanah-marshal.list
+
 -include $(top_srcdir)/git.mk
diff --git a/src/almanah-marshal.list b/src/almanah-marshal.list
new file mode 100644
index 0000000..72f9937
--- /dev/null
+++ b/src/almanah-marshal.list
@@ -0,0 +1 @@
+VOID:STRING,STRING
diff --git a/src/main.c b/src/main.c
index dbeaf96..aeefdd0 100644
--- a/src/main.c
+++ b/src/main.c
@@ -31,8 +31,23 @@
 Almanah *almanah;
 
 static G_GNUC_NORETURN void
-storage_manager_disconnected_cb (AlmanahStorageManager *self, gpointer user_data)
+storage_manager_disconnected_cb (AlmanahStorageManager *self, const gchar *gpgme_error_message, const gchar *warning_message, gpointer user_data)
 {
+	if (gpgme_error_message != NULL || warning_message != NULL) {
+		GtkWidget *dialog = gtk_message_dialog_new (NULL, GTK_DIALOG_MODAL, GTK_MESSAGE_ERROR, GTK_BUTTONS_OK,
+							    _("Error encrypting database"));
+
+		if (gpgme_error_message != NULL && warning_message != NULL)
+			gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), "%s %s", warning_message, gpgme_error_message);
+		else if (gpgme_error_message != NULL)
+			gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), "%s", gpgme_error_message);
+		else
+			gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), "%s", warning_message);
+
+		gtk_dialog_run (GTK_DIALOG (dialog));
+		gtk_widget_destroy (dialog);
+	}
+
 	g_object_unref (almanah->storage_manager);
 	g_object_unref (almanah->gconf_client);
 	g_object_unref (almanah->page_setup);
@@ -49,17 +64,8 @@ storage_manager_disconnected_cb (AlmanahStorageManager *self, gpointer user_data
 void
 almanah_quit (void)
 {
-	GError *error = NULL;
-
 	g_signal_connect (almanah->storage_manager, "disconnected", G_CALLBACK (storage_manager_disconnected_cb), NULL);
-	if (almanah_storage_manager_disconnect (almanah->storage_manager, &error) == FALSE) {
-		GtkWidget *dialog = gtk_message_dialog_new (GTK_WINDOW (almanah->main_window),
-							    GTK_DIALOG_MODAL, GTK_MESSAGE_ERROR, GTK_BUTTONS_OK,
-							    _("Error closing database"));
-		gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), "%s", error->message);
-		gtk_dialog_run (GTK_DIALOG (dialog));
-		gtk_widget_destroy (dialog);
-	}
+	almanah_storage_manager_disconnect (almanah->storage_manager, NULL);
 
 	if (almanah->add_definition_dialog != NULL)
 		gtk_widget_destroy (almanah->add_definition_dialog);
@@ -76,12 +82,7 @@ almanah_quit (void)
 	g_object_unref (almanah->event_manager);
 
 	/* Quitting is actually done in storage_manager_disconnected_cb, which is called once
-	 * the storage manager has encrypted the DB and disconnected from it.
-	 * Unless, that is, disconnection failed. */
-	if (error != NULL) {
-		g_error_free (error);
-		storage_manager_disconnected_cb (almanah->storage_manager, NULL);
-	}
+	 * the storage manager has encrypted the DB and disconnected from it. */
 }
 
 int
diff --git a/src/storage-manager.c b/src/storage-manager.c
index 43c2287..4d22da0 100644
--- a/src/storage-manager.c
+++ b/src/storage-manager.c
@@ -33,6 +33,7 @@
 #include "entry.h"
 #include "definition.h"
 #include "storage-manager.h"
+#include "almanah-marshal.h"
 
 static void almanah_storage_manager_finalize (GObject *object);
 static void almanah_storage_manager_get_property (GObject *object, guint property_id, GValue *value, GParamSpec *pspec);
@@ -87,8 +88,8 @@ almanah_storage_manager_class_init (AlmanahStorageManagerClass *klass)
 				G_TYPE_FROM_CLASS (klass),
 				G_SIGNAL_RUN_LAST,
 				0, NULL, NULL,
-				g_cclosure_marshal_VOID__OBJECT,
-				G_TYPE_NONE, 1, ALMANAH_TYPE_STORAGE_MANAGER);
+				almanah_marshal_VOID__STRING_STRING,
+				G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_STRING);
 	storage_manager_signals[SIGNAL_DEFINITION_ADDED] = g_signal_new ("definition-added",
 				G_TYPE_FROM_CLASS (klass),
 				G_SIGNAL_RUN_LAST,
@@ -310,23 +311,24 @@ database_idle_cb (CipherOperation *operation)
 
 	if (gpgme_wait (operation->context, &error_gpgme, FALSE) != NULL || error_gpgme != GPG_ERR_NO_ERROR) {
 		struct stat db_stat;
-
-		g_warning (_("Error encrypting database: %s"), gpgme_strerror (error_gpgme));
+		gchar *warning_message = NULL;
 
 		/* Check to see if the encrypted file is 0B in size, which isn't good.
 		 * Not much we can do about it except quit without deleting the plaintext database. */
 		g_stat (self->priv->filename, &db_stat);
 		if (g_file_test (self->priv->filename, G_FILE_TEST_IS_REGULAR) == FALSE || db_stat.st_size == 0) {
-			g_warning (_("Error encrypting database: %s"),
-				   _("The encrypted database is empty. The plain database file has been left undeleted as backup."));
+			warning_message = g_strdup (_("The encrypted database is empty. The plain database file has been left undeleted as backup."));
 		} else if (g_unlink (self->priv->plain_filename) != 0) {
 			/* Delete the plain file */
-			g_warning (_("Could not delete plain database file \"%s\"."), self->priv->plain_filename);
+			warning_message = g_strdup_printf (_("Could not delete plain database file \"%s\"."), self->priv->plain_filename);
 		}
 
 		/* A slight assumption that we're disconnecting at this point (we're technically
 		 * only encrypting), but a valid one. */
-		g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0, self);
+		g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0,
+			       (error_gpgme == GPG_ERR_NO_ERROR) ? NULL: gpgme_strerror (error_gpgme),
+			       warning_message);
+		g_free (warning_message);
 
 		/* Finished! */
 		cipher_operation_free (operation);
@@ -513,7 +515,6 @@ almanah_storage_manager_connect (AlmanahStorageManager *self, GError **error)
 #else
 	/* Make a backup of the plaintext database file */
 	back_up_file (self->priv->plain_filename);
-
 	self->priv->decrypted = FALSE;
 #endif /* ENABLE_ENCRYPTION */
 
@@ -543,37 +544,38 @@ almanah_storage_manager_disconnect (AlmanahStorageManager *self, GError **error)
 	/* Close the DB connection */
 	sqlite3_close (self->priv->connection);
 
+#ifdef ENABLE_ENCRYPTION
+	/* If the database wasn't encrypted before we opened it, we won't encrypt it when closing */
 	if (self->priv->decrypted == FALSE) {
-		g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0, self);
+		g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0, NULL, NULL);
 		return TRUE;
 	}
 
-#ifdef ENABLE_ENCRYPTION
+	/* Get the encryption key */
 	encryption_key = get_encryption_key ();
 	if (encryption_key == NULL) {
-		g_message (_("Error getting encryption key: GConf key \"%s\" invalid or empty. Your diary will not be encrypted; please install Seahorse and set up a default key, or ignore this message."), ENCRYPTION_KEY_GCONF_PATH);
-		g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0, self);
+		/* The preferences are set to not encrypt the diary */
+		g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0, NULL, NULL);
 		return TRUE;
 	}
 
 	/* Encrypt the plain DB file */
 	if (encrypt_database (self, encryption_key, &child_error) != TRUE) {
-		if (child_error->code != ALMANAH_STORAGE_MANAGER_ERROR_GETTING_KEY) {
-			/* Propagate the error */
-			g_propagate_error (error, child_error);
-			return FALSE;
-		}
+		g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0, NULL, child_error->message);
 
-		/* Log an error about being unable to get the key
-		 * then continue without encrypting. */
-		g_warning ("%s", child_error->message);
-		g_error_free (child_error);
+		if (g_error_matches (child_error, ALMANAH_STORAGE_MANAGER_ERROR, ALMANAH_STORAGE_MANAGER_ERROR_GETTING_KEY) == TRUE)
+			g_propagate_error (error, child_error);
+		else
+			g_error_free (child_error);
 
-		g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0, self);
+		g_free (encryption_key);
+		return FALSE;
 	}
 
 	g_free (encryption_key);
-#endif /* ENABLE_ENCRYPTION */
+#else /* ENABLE_ENCRYPTION */
+	g_signal_emit (self, storage_manager_signals[SIGNAL_DISCONNECTED], 0, NULL, NULL);
+#endif /* !ENABLE_ENCRYPTION */
 
 	return TRUE;
 }