/cpw/tomkiewicz/masterpassword: 52b91b4b8314: Proper error handl...

Tomasz Wasilczyk tomkiewicz at cpw.pidgin.im
Fri Mar 22 17:39:25 EDT 2013


Changeset: 52b91b4b8314938f4f05e332ddc2b2c2dd5bff45
Author:	 Tomasz Wasilczyk <tomkiewicz at cpw.pidgin.im>
Date:	 2013-03-22 22:39 +0100
Branch:	 soc.2008.masterpassword
URL: https://hg.pidgin.im/cpw/tomkiewicz/masterpassword/rev/52b91b4b8314

Description:

Proper error handling when doing a password migration, fixes reverting it.

diffstat:

 libpurple/keyring.c                          |  31 +++++++++++++--------------
 libpurple/plugins/keyrings/gnomekeyring.c    |  20 +++++------------
 libpurple/plugins/keyrings/internalkeyring.c |  30 ++++++++++++++++++--------
 libpurple/plugins/keyrings/kwallet.cpp       |  14 ++---------
 libpurple/plugins/keyrings/secretservice.c   |  19 ++++------------
 5 files changed, 50 insertions(+), 64 deletions(-)

diffs (truncated from 343 to 300 lines):

diff --git a/libpurple/keyring.c b/libpurple/keyring.c
--- a/libpurple/keyring.c
+++ b/libpurple/keyring.c
@@ -419,8 +419,6 @@ purple_keyring_set_inuse_check_error_cb(
 
 	tracker = (PurpleKeyringChangeTracker *)data;
 
-	g_return_if_fail(tracker->abort == FALSE);
-
 	tracker->read_outstanding--;
 
 	name = purple_account_get_username(account);
@@ -431,34 +429,37 @@ purple_keyring_set_inuse_check_error_cb(
 		switch(error->code) {
 			case PURPLE_KEYRING_ERROR_NOCAP:
 				purple_debug_info("keyring",
-					"Keyring could not save password for account %s: %s.\n",
+					"Keyring does not support saving a password for account %s: %s.\n",
 					name, error->message);
 				break;
 
 			case PURPLE_KEYRING_ERROR_NOPASSWD:
-				purple_debug_info("keyring",
-					"No password found while changing keyring for account %s: %s.\n",
-					name, error->message);
+				if (purple_debug_is_verbose()) {
+					purple_debug_misc("keyring",
+						"No password found while changing keyring for account %s: %s.\n",
+						name, error->message);
+				}
 				break;
 
 			case PURPLE_KEYRING_ERROR_NOCHANNEL:
-				purple_debug_info("keyring",
+				purple_debug_error("keyring",
 					"Failed to communicate with backend while changing keyring for account %s: %s. Aborting changes.\n",
 					name, error->message);
 				tracker->abort = TRUE;
 				break;
 
 			default:
-				purple_debug_info("keyring",
-					"Unknown error while changing keyring for account %s: %s.\n",
+				purple_debug_error("keyring",
+					"Unknown error while changing keyring for account %s: %s. Aborting changes.\n",
 					name, error->message);
+				tracker->abort = TRUE;
 				break;
 		}
 	}
 
 	/* if this was the last one */
-	if (tracker->finished == TRUE && tracker->read_outstanding == 0) {
-		if (tracker->abort == TRUE && tracker->force == FALSE) {
+	if ((tracker->finished && tracker->read_outstanding == 0) || (tracker->abort && !tracker->force)) {
+		if (tracker->abort && !tracker->force) {
 			if (tracker->cb != NULL)
 				tracker->cb(tracker->old, FALSE, tracker->error, tracker->data);
 
@@ -468,7 +469,7 @@ purple_keyring_set_inuse_check_error_cb(
 			if (close != NULL)
 				close(NULL);
 
-			purple_debug_info("keyring",
+			purple_debug_error("keyring",
 				"Failed to change keyring, aborting.\n");
 
 			purple_notify_error(NULL, _("Keyrings"), _("Failed to change the keyring."),
@@ -526,14 +527,12 @@ purple_keyring_set_inuse_got_pw_cb(Purpl
 		if (error->code == PURPLE_KEYRING_ERROR_NOPASSWD ||
 		    error->code == PURPLE_KEYRING_ERROR_NOACCOUNT ||
 		    tracker->force == TRUE) {
-			/* don't save password, and directly trigger callback */
-			purple_keyring_set_inuse_check_error_cb(account, error, data);
-
+			/* don't save password, and ignore it */
 		} else {
 			/* fatal error, abort all */
 			tracker->abort = TRUE;
 		}
-
+		purple_keyring_set_inuse_check_error_cb(account, error, data);
 	} else {
 		save = purple_keyring_get_save_password(new);
 
diff --git a/libpurple/plugins/keyrings/gnomekeyring.c b/libpurple/plugins/keyrings/gnomekeyring.c
--- a/libpurple/plugins/keyrings/gnomekeyring.c
+++ b/libpurple/plugins/keyrings/gnomekeyring.c
@@ -41,8 +41,6 @@
 #define GNOMEKEYRING_AUTHOR      "Scrouaf (scrouaf[at]soc.pidgin.im)"
 #define GNOMEKEYRING_ID          "keyring-gnomekeyring"
 
-#define ERR_GNOMEKEYRINGPLUGIN 	gkp_error_domain()
-
 static PurpleKeyring *keyring_handler = NULL;
 
 typedef struct _InfoStorage InfoStorage;
@@ -54,12 +52,6 @@ struct _InfoStorage
 	gpointer user_data;
 };
 
-static GQuark gkp_error_domain(void)
-{
-	return g_quark_from_static_string("Gnome-Keyring plugin");
-}
-
-
 /***********************************************/
 /*     Keyring interface                       */
 /***********************************************/
@@ -77,7 +69,7 @@ gkp_read_continue(GnomeKeyringResult res
 	if (result != GNOME_KEYRING_RESULT_OK) {
 		switch(result) {
 			case GNOME_KEYRING_RESULT_NO_MATCH:
-				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+				error = g_error_new(PURPLE_KEYRING_ERROR,
 					PURPLE_KEYRING_ERROR_NOPASSWD,
 					"No password found for account : %s",
 					purple_account_get_username(account));
@@ -88,7 +80,7 @@ gkp_read_continue(GnomeKeyringResult res
 
 			case GNOME_KEYRING_RESULT_NO_KEYRING_DAEMON:
 			case GNOME_KEYRING_RESULT_IO_ERROR:
-				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+				error = g_error_new(PURPLE_KEYRING_ERROR,
 					PURPLE_KEYRING_ERROR_NOCHANNEL,
 					"Failed to communicate with GNOME Keyring (account : %s).",
 					purple_account_get_username(account));
@@ -98,7 +90,7 @@ gkp_read_continue(GnomeKeyringResult res
 				return;
 
 			default:
-				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+				error = g_error_new(PURPLE_KEYRING_ERROR,
 					PURPLE_KEYRING_ERROR_NOCHANNEL,
 					"Unknown error (account : %s).",
 					purple_account_get_username(account));
@@ -154,7 +146,7 @@ gkp_save_continue(GnomeKeyringResult res
 					"Could not update password for %s (%s) : not found.\n",
 					purple_account_get_username(account),
 					purple_account_get_protocol_id(account));
-				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+				error = g_error_new(PURPLE_KEYRING_ERROR,
 					PURPLE_KEYRING_ERROR_NOPASSWD,
 					"Could not update password for %s : not found",
 					purple_account_get_username(account));
@@ -169,7 +161,7 @@ gkp_save_continue(GnomeKeyringResult res
 					"Failed to communicate with GNOME Keyring (account : %s (%s)).\n",
 					purple_account_get_username(account),
 					purple_account_get_protocol_id(account));
-				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+				error = g_error_new(PURPLE_KEYRING_ERROR,
 					PURPLE_KEYRING_ERROR_NOCHANNEL,
 					"Failed to communicate with GNOME Keyring (account : %s).",
 					purple_account_get_username(account));
@@ -183,7 +175,7 @@ gkp_save_continue(GnomeKeyringResult res
 					"Unknown error (account : %s (%s)).\n",
 					purple_account_get_username(account),
 					purple_account_get_protocol_id(account));
-				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+				error = g_error_new(PURPLE_KEYRING_ERROR,
 					PURPLE_KEYRING_ERROR_NOCHANNEL,
 					"Unknown error (account : %s).",
 					purple_account_get_username(account));
diff --git a/libpurple/plugins/keyrings/internalkeyring.c b/libpurple/plugins/keyrings/internalkeyring.c
--- a/libpurple/plugins/keyrings/internalkeyring.c
+++ b/libpurple/plugins/keyrings/internalkeyring.c
@@ -79,10 +79,12 @@ internal_keyring_read(PurpleAccount *acc
 		if (cb != NULL)
 			cb(account, password, NULL, data);
 	} else {
-		purple_debug_misc("keyring-internal",
-			"No password for account %s (%s).\n",
-			purple_account_get_username(account),
-			purple_account_get_protocol_id(account));
+		if (purple_debug_is_verbose()) {
+			purple_debug_misc("keyring-internal",
+				"No password for account %s (%s).\n",
+				purple_account_get_username(account),
+				purple_account_get_protocol_id(account));
+		}
 		error = g_error_new(PURPLE_KEYRING_ERROR,
 			PURPLE_KEYRING_ERROR_NOPASSWD, "Password not found.");
 		if (cb != NULL)
@@ -95,8 +97,11 @@ static void
 internal_keyring_save(PurpleAccount *account, const gchar *password,
 	PurpleKeyringSaveCallback cb, gpointer data)
 {
+	void *old_password;
 	internal_keyring_open();
 
+	old_password = g_hash_table_lookup(internal_keyring_passwords, account);
+
 	if (password == NULL)
 		g_hash_table_remove(internal_keyring_passwords, account);
 	else {
@@ -104,11 +109,18 @@ internal_keyring_save(PurpleAccount *acc
 			g_strdup(password));
 	}
 
-	purple_debug_misc("keyring-internal",
-		"Password %s for account %s (%s).\n",
-		(password == NULL ? "removed" : "saved"),
-		purple_account_get_username(account),
-		purple_account_get_protocol_id(account));
+	if (!(password == NULL && old_password == NULL)) {
+		purple_debug_misc("keyring-internal",
+			"Password %s for account %s (%s).\n",
+			(password == NULL ? "removed" : (old_password == NULL ? "saved" : "updated")),
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+	} else if (purple_debug_is_verbose()) {
+		purple_debug_misc("keyring-internal",
+			"Password for account %s (%s) was already removed.\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+	}
 
 	if (cb != NULL)
 		cb(account, NULL, data);
diff --git a/libpurple/plugins/keyrings/kwallet.cpp b/libpurple/plugins/keyrings/kwallet.cpp
--- a/libpurple/plugins/keyrings/kwallet.cpp
+++ b/libpurple/plugins/keyrings/kwallet.cpp
@@ -50,8 +50,6 @@
 PurpleKeyring *keyring_handler = NULL;
 QCoreApplication *qCoreApp = NULL;
 
-#define ERR_KWALLETPLUGIN kwallet_plugin_error_domain()
-
 namespace KWalletPlugin {
 
 class request
@@ -124,12 +122,6 @@ class read_request : public request
 		PurpleKeyringReadCallback callback;
 };
 
-static GQuark
-kwallet_plugin_error_domain(void)
-{
-	return g_quark_from_static_string("KWallet keyring");
-}
-
 }
 
 static gboolean
@@ -147,7 +139,7 @@ KWalletPlugin::request::~request()
 void
 KWalletPlugin::request::abort()
 {
-	detailedAbort(PURPLE_KEYRING_ERROR_UNKNOWN);
+	detailedAbort(PURPLE_KEYRING_ERROR_NOCHANNEL);
 }
 
 KWalletPlugin::engine::engine()
@@ -342,7 +334,7 @@ KWalletPlugin::save_request::detailedAbo
 	if (callback == NULL)
 		return;
 
-	gerror = g_error_new(ERR_KWALLETPLUGIN, error,
+	gerror = g_error_new(PURPLE_KEYRING_ERROR, error,
 		"Failed to save password");
 	callback(account, gerror, data);
 	g_error_free(gerror);
@@ -355,7 +347,7 @@ KWalletPlugin::read_request::detailedAbo
 	if (callback == NULL)
 		return;
 
-	gerror = g_error_new(ERR_KWALLETPLUGIN, error,
+	gerror = g_error_new(PURPLE_KEYRING_ERROR, error,
 		"Failed to read password");
 	callback(account, NULL, gerror, data);
 	g_error_free(gerror);
diff --git a/libpurple/plugins/keyrings/secretservice.c b/libpurple/plugins/keyrings/secretservice.c
--- a/libpurple/plugins/keyrings/secretservice.c
+++ b/libpurple/plugins/keyrings/secretservice.c
@@ -33,8 +33,6 @@
 #define SECRETSERVICE_NAME        N_("Secret Service")
 #define SECRETSERVICE_ID          "keyring-libsecret"
 
-#define ERR_SECRETSERVICEPLUGIN   (ss_error_domain())
-
 static PurpleKeyring *keyring_handler = NULL;
 
 static const SecretSchema purple_schema = {
@@ -57,13 +55,6 @@ struct _InfoStorage
 	gpointer user_data;
 };
 
-static GQuark
-ss_error_domain(void)
-{
-	return g_quark_from_static_string("SecretService plugin");
-}
-
-
 /***********************************************/
 /*     Keyring interface                       */
 /***********************************************/
@@ -85,7 +76,7 @@ ss_read_continue(GObject *object, GAsync
 		switch (code) {



More information about the Commits mailing list