soc.2008.masterpassword: 90a6d065: Fixed problem in async purple_peyring_se...

scrouaf at soc.pidgin.im scrouaf at soc.pidgin.im
Fri Jul 11 18:30:59 EDT 2008


-----------------------------------------------------------------
Revision: 90a6d065106178c55b2ba651f408e82d589a55aa
Ancestor: c9da84e0afb5696de10cd93b3eeb2b5dd37b7b75
Author: scrouaf at soc.pidgin.im
Date: 2008-07-11T22:26:24
Branch: im.pidgin.soc.2008.masterpassword
URL: http://d.pidgin.im/viewmtn/revision/info/90a6d065106178c55b2ba651f408e82d589a55aa

Modified files:
        libpurple/keyring.c libpurple/keyring.h
        libpurple/plugins/keyrings/internalkeyring.c

ChangeLog: 

Fixed problem in async purple_peyring_set_inuse()

-------------- next part --------------
============================================================
--- libpurple/keyring.c	a29ee4ffdd7bbbcde64e133e8f4d21b67b0148f1
+++ libpurple/keyring.c	b6a848dc1adaf30bb204a3715524e197b26c651a
@@ -31,6 +31,7 @@
  *  - accessors
  *
  * TODO :
+ *  - unregister
  *  - use accessors
  *  - purple_keyring_init()
  *  - purple_keyring_set_inuse() needs to be async for error checking an reversability.
@@ -78,19 +79,34 @@ const GList * 
 /* manipulate keyring list, used by config interface */
 
 const GList * 
-purple_keyring_get_keyringlist(void)
+purple_keyring_get_keyringlist()
 {
 	return purple_keyring_keyringlist;
 }
 
 const PurpleKeyring * 
-purple_keyring_get_inuse(void)
+purple_keyring_get_inuse()
 {
 	return purple_keyring_inuse;
 }
 
 
-//typedef void (*PurpleKeyringSaveCallback)(const PurpleAccount * account, GError ** error, gpointer data);
+/**
+ * If reading fails, export empty, issue warning, continue
+ * If writing fails, abort.
+ */
+struct _PurpleKeyringChangeTracker
+{
+	GError ** error;			// could probably be dropped
+	PurpleKeyringSetInUseCb cb;
+	gpointer data;
+	PurpleKeyring * new;
+	PurpleKeyring * old;		// we are done when : finished == TRUE && read_outstanding == 0
+	int read_outstanding;
+	gboolean finished;
+	gboolean abort;
+};
+
 void
 purple_keyring_set_inuse_check_error_cb(const PurpleAccount * account,
 					GError ** error,
@@ -98,7 +114,11 @@ purple_keyring_set_inuse_check_error_cb(
 {
 
 	const char * name;
+	PurpleKeyringClose close;
+	struct _PurpleKeyringChangeTracker * tracker;
 
+	tracker = (struct _PurpleKeyringChangeTracker *)data;
+
 	name = purple_account_get_username(account);
 
 	if ((error != NULL) && ((**error).domain == ERR_PIDGINKEYRING)) {
@@ -114,20 +134,43 @@ purple_keyring_set_inuse_check_error_cb(
 				break;
 
 			case ERR_NOCHANNEL :
-				g_debug("Failed to communicate with backend while changing keyring for account %s", name);
+				g_debug("Failed to communicate with backend while changing keyring for account %s, aborting change.", name);
+				tracker->abort == TRUE;
 				break;
-				/* FIXME : this should somehow abort the whole procedure */
 
 			default :
+				// FIXME : display error string
 				g_debug("Unknown error while changing keyring for account %s", name);
 				break;
 		}
 	}
 
+	/* if this was the last one */
+	if (tracker->finished == TRUE) && (tracker->read_outstanding == 0)) {
+	
+		if (tracker->abort == TRUE) {
+			
+			tracker->abort = TRUE;
+
+			close = purple_keyring_get_close_keyring(tracker->old);
+			close(error);			
+
+			g_free(tracker);
+			return;
+		} else {
+			close = purple_keyring_get_close_keyring(tracker->new);
+			close(error);
+
+			tracker->cb(TRUE, error, tracker->data);
+			g_free(tracker);
+			return;
+		}
+
+	}
 	return;
 }
 
-//typedef void (*PurpleKeyringReadCallback)(const PurpleAccount * account, gchar * password, GError * error, gpointer data);
+
 void
 purple_keyring_set_inuse_got_pw_cb(const PurpleAccount * account, 
 			 gchar * password, 
@@ -136,51 +179,72 @@ purple_keyring_set_inuse_got_pw_cb(const
 {
 	PurpleKeyring * new;
 	PurpleKeyringSave save;
-	new = (PurpleKeyring *)data;
-	/* XXX check for read error or just forward ? */
+	struct _PurpleKeyringChangeTracker * tracker;
 
-	/* XXX change to use accessor */
+	
+	tracker = (struct _PurpleKeyringChangeTracker *)data;
+	new = tracker->new;
 
-	//typedef void (*PurpleKeyringSave)(const PurpleAccount * account, gchar * password, GError ** error, PurpleKeyringSaveCallback cb, gpointer data);
+	/* XXX check for read error or just forward ? */
 
+	tracker->read_outstanding--;
+	
 	save = purple_keyring_get_save_password(new);
 	save(account, password, error, purple_keyring_set_inuse_check_error_cb, 
-		NULL);
+		tracker);
 
 	return;
 }
 
 /* FIXME : needs to be async and cancelable */
+/* PurpleKeyringSetInUseCb */
 void 
 purple_keyring_set_inuse(PurpleKeyring * new,
-			 GError ** error)
+			 GError ** error,
+			 PurpleKeyringSetInUseCb cb,
+			 gpointer data)
 {
 
 	GList * cur;
 	const PurpleKeyring * old;
-	PurpleKeyringClose close;
 	PurpleKeyringRead read;
+	struct _PurpleKeyringChangeTracker * tracker;
 
+
 	if (purple_keyring_inuse != NULL) {
 
+		tracker = g_malloc(sizeof(struct _PurpleKeyringChangeTracker));
 		old = purple_keyring_get_inuse();
 
-		for (cur = purple_accounts_get_all(); cur != NULL; cur = cur->next)
+		tracker->error = error;
+		tracker->cb = cb;
+		tracker->data = data;
+		tracker->new = new;
+		tracker->old = old;
+		tracker->read_outstanding = 0;
+		tracker->finished = FALSE;
+		tracker->abort = FALSE;
+
+		for (cur = purple_accounts_get_all(); 
+		    (cur != NULL) && (tracker->abort != TRUE);
+		    cur = cur->next)
 		{
+			tracker->read_outstanding++;
+
+			if (cur->next == NULL) {
+				tracker->finished = TRUE;
+			}
+
 			read = purple_keyring_get_read_password(old);
-			read(cur->data, NULL, purple_keyring_set_inuse_got_pw_cb, (void*)new);
+			read(cur->data, error, purple_keyring_set_inuse_got_pw_cb, tracker);
 		}
 
-		/* FIXME :
-		 * What happens if safe is closed before passwords have been successfully stored ? 
-		 */
+	} else { /* no keyring was set before. */
 
-		close = purple_keyring_get_close_keyring(old);
-		close(error);	/* should automatically free all passwords */
+		purple_keyring_inuse = new;
+		cb(data);
+		return;
 	}
-
-	purple_keyring_inuse = new;
-	return;
 }
 
 /* register a keyring plugin */
@@ -234,7 +298,7 @@ purple_keyring_export_password(PurpleAcc
 void
 purple_keyring_export_password(PurpleAccount * account,
 			       GError ** error,
-			       PurpleKeyringImportCallback cb,
+			       PurpleKeyringExportCallback cb,
 			       gpointer data)
 {
 	PurpleKeyringExportPassword export;
@@ -243,7 +307,7 @@ purple_keyring_export_password(PurpleAcc
 
 		g_set_error(error, ERR_PIDGINKEYRING, ERR_NOKEYRING, 
 			"No Keyring configured.");
-		cb(error, data);
+		cb(NULL, error, data);
 
 	} else {
 		export = purple_keyring_get_export_password(purple_keyring_inuse);
@@ -414,7 +478,7 @@ PurpleKeyringPasswordNode * 
 	/* PurpleKeyringPasswordNode */
 
 PurpleKeyringPasswordNode * 
-purple_keyring_password_node_new(void)
+purple_keyring_password_node_new()
 {
 	PurpleKeyringPasswordNode * ret;
 
@@ -487,7 +551,7 @@ GQuark
 }
 
 GQuark
-purple_keyring_error_domain(void)
+purple_keyring_error_domain()
 {
 	return g_quark_from_static_string("Libpurple keyring");
 }
============================================================
--- libpurple/keyring.h	de44b58d985308b68a39b9db9305e6274fd20ea0
+++ libpurple/keyring.h	9a29acd9a33d03fbeea0577809e7f488fd47e038
@@ -1,6 +1,8 @@
 /**
  * @file keyring.h Keyring plugin API
  * @ingroup core
+ *
+ * @todo
  */
 
 /* purple
@@ -20,7 +22,7 @@
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
+ * along with this program ; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02111-1301  USA
  */
 
@@ -34,24 +36,63 @@
  * DATA STRUCTURES **************************************
  ********************************************************/
 
-typedef struct _PurpleKeyring PurpleKeyring;	/* TODO : move back as public struct */
+typedef struct _PurpleKeyring PurpleKeyring;				/* public (for now) */
 typedef struct _PurpleKeyringPasswordNode PurpleKeyringPasswordNode;	/* opaque struct */
 
+/*
+ * XXX maybe strip a couple GError* if they're not used,
+ * since they should only be interresting for the callback
+ *  --> ability to forward errors ?
+ *
+ */
 
+/*********************************************************/
+/** @name Callbacks for basic keyring operation          */
+/*********************************************************/
 
 /**
- * XXX maybe strip a couple GError* if they're not used 
- * since they should only be interresting for the callback
- *  --> ability to forward errors ?
+ * Callback for once a password is read. If there was a problem, the password should
+ * be NULL, and the error set.
+ *
+ * @param account The account of which the password was asked.
+ * @param password The password that was read
+ * @param error Error that could have occured. Must be freed if non NULL.
+ * @param data Data passed to the callback.
  */
-
-/* callbacks */
 typedef void (*PurpleKeyringReadCallback)(const PurpleAccount * account,
 					  gchar * password,
 					  GError ** error,
 					  gpointer data);
-typedef void (*PurpleKeyringSaveCallback)(const PurpleAccount * account, GError ** error, gpointer data);
-typedef void (*PurpleKeyringChangeMasterCallback)(int result, GError * error, gpointer data);
+
+/**
+ * Callback for once a password has been stored. If there was a problem, the error will be set.
+ *
+ * @param account The account of which the password was saved.
+ * @param error Error that could have occured. Must be freed if non NULL.
+ * @param data Data passed to the callback.
+ */
+typedef void (*PurpleKeyringSaveCallback)(const PurpleAccount * account, 
+					  GError ** error,
+					  gpointer data);
+
+/**
+ * Callback for once the master password for a keyring has been changed.
+ *
+ * @param result Will be TRUE if the password has been changed, false otherwise.
+ * @param error Error that has occured. Must be freed if non NULL.
+ * @param data Data passed to the callback.
+ */
+typedef void (*PurpleKeyringChangeMasterCallback)(gboolean result,
+						  GError ** error,
+						  gpointer data);
+
+/**
+ * Callback for once the master password for a keyring has been changed.
+ *
+ * @param result Will be TRUE if the password has been changed, false otherwise.
+ * @param error Error that has occured. Must be freed if non NULL.
+ * @param data Data passed to the callback.
+ */
 typedef void (*PurpleKeyringImportCallback)(GError ** error, gpointer data);	/* XXX add a gboolean result or just use error ? */
 typedef void (*PurpleKeyringExportCallback)(PurpleKeyringPasswordNode * result, GError ** error, gpointer data);
 
@@ -66,14 +107,7 @@ typedef void (*PurpleKeyringFree)(gchar 
 typedef void (*PurpleKeyringChangeMaster)(GError ** error, PurpleKeyringChangeMasterCallback cb, gpointer data);
 typedef void (*PurpleKeyringFree)(gchar * password, GError ** error);
 
-/**
- * TODO : 
- *  - add GErrors in there
- *  - add callback, it needs to be async
- *  - typedefs for callbacks
- */
 typedef void (*PurpleKeyringImportPassword)(const PurpleKeyringPasswordNode * nodeinfo, GError ** error, PurpleKeyringImportCallback cb, gpointer data);
-
 typedef void (*PurpleKeyringExportPassword)(const PurpleAccount * account,GError ** error, PurpleKeyringExportCallback cb,     gpointer data);
 
 /* information about a keyring */
@@ -100,8 +134,8 @@ struct _PurpleKeyring
 /***************************************/
 
 /* manipulate keyring list, used by config interface */
-const GList * purple_keyring_get_keyringlist(void);
-const PurpleKeyring * purple_keyring_get_inuse(void);
+const GList * purple_keyring_get_keyringlist();
+const PurpleKeyring * purple_keyring_get_inuse();
 
 // FIXME : needs to be async so it can detect errors and undo changes
 void
@@ -115,7 +149,7 @@ void purple_plugin_keyring_register(Purp
 void purple_plugin_keyring_register(PurpleKeyring * info);
 
 /* used by account.c while reading a password from xml */
-gboolean purple_keyring_import_password(const PurpleKeyringPasswordNode * passwordnode, 
+void purple_keyring_import_password(const PurpleKeyringPasswordNode * passwordnode, 
 					GError ** error, 
 					PurpleKeyringImportCallback cb, 
 					gpointer data);
@@ -125,7 +159,7 @@ void purple_keyring_export_password(Purp
  */
 void purple_keyring_export_password(PurpleAccount * account,
 				    GError ** error,
-				    PurpleKeyringImportCallback cb,
+				    PurpleKeyringExportCallback cb,
 				    gpointer data);
 
 
@@ -170,7 +204,7 @@ void purple_keyring_set_export_password(
 
 	/* PurpleKeyringPasswordNode */
 
-PurpleKeyringPasswordNode * purple_keyring_password_node_new(void);
+PurpleKeyringPasswordNode * purple_keyring_password_node_new();
 void purple_keyring_password_node_delete(PurpleKeyringPasswordNode * node);
 
 const PurpleAccount * 
@@ -192,19 +226,25 @@ void purple_keyring_init();
 /** @name Error Codes                  */
 /***************************************/
 
+/**
+ * Error domain GQuark. 
+ * See @ref purple_keyring_error_domain .
+ */
 #define ERR_PIDGINKEYRING 	purple_keyring_error_domain()
-GQuark purple_keyring_error_domain(void);
+/** stuff here too */
+GQuark purple_keyring_error_domain();
 
-enum
+/** error codes for keyrings. */
+enum PurpleKeyringError
 {
-	ERR_OK = 0,		/* no error */
-	ERR_NOPASSWD = 1,	/* no stored password */
-	ERR_NOACCOUNT,		/* account not found */
-	ERR_WRONGPASS,		/* user submitted wrong password when prompted */
-	ERR_WRONGFORMAT,	/* data passed is not in suitable format*/
-	ERR_NOKEYRING,		/* no keyring configured */
+	ERR_OK = 0,		/**< no error. */
+	ERR_NOPASSWD = 1,	/**< no stored password. */
+	ERR_NOACCOUNT,		/**< account not found. */
+	ERR_WRONGPASS,		/**< user submitted wrong password when prompted. */
+	ERR_WRONGFORMAT,	/**< data passed is not in suitable format. */
+	ERR_NOKEYRING,		/**< no keyring configured. */
 	ERR_NOCHANNEL
-} PurpleKeyringError;
+};
 
 
 #endif /* _PURPLE_KEYRING_H_ */
============================================================
--- libpurple/plugins/keyrings/internalkeyring.c	175baecfbfd72167d9bc7af89ebbc2717a7c1710
+++ libpurple/plugins/keyrings/internalkeyring.c	4df58eb1a06f0e9ff3d91edb0b5695e053666763
@@ -1,9 +1,8 @@
 /* TODO
  - fix error reporting
- - uses accessors for PurpleKeyringPasswordNode
  - use hashtable instead of Glib
  - plugin interface
- - keyring info struct
+ - move keyring info struct upwards
 */
 
 #include <glib.h>
@@ -275,9 +274,7 @@ InternalKeyring_import_password(const Pu
 /**
  * Exports password info to a PurpleKeyringPasswordNode structure
  * (called for each account when accounts are synced)
- * TODO : add error reporting 
- *	  use accessors for PurpleKeyringPasswordNode (FIXME)
- *	  FIXME : REWRITE AS ASYNC
+ * TODO : add proper error reporting 
  */
 void
 InternalKeyring_export_password(const PurpleAccount * account,
@@ -293,6 +290,7 @@ InternalKeyring_export_password(const Pu
 
 	if (pwinfo->password == NULL) {
 
+		// FIXME : error
 		cb(NULL, error, data);
 		return;
 


More information about the Commits mailing list