Fri, 11 Jul 2008 22:26:24 +0000
Fixed problem in async purple_peyring_set_inuse()
--- a/libpurple/keyring.c Thu Jul 10 20:56:48 2008 +0000 +++ b/libpurple/keyring.c Fri Jul 11 22:26:24 2008 +0000 @@ -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 @@ /* 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,6 +114,10 @@ { const char * name; + PurpleKeyringClose close; + struct _PurpleKeyringChangeTracker * tracker; + + tracker = (struct _PurpleKeyringChangeTracker *)data; name = purple_account_get_username(account); @@ -114,20 +134,43 @@ 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 @@ { PurpleKeyring * new; PurpleKeyringSave save; - new = (PurpleKeyring *)data; + struct _PurpleKeyringChangeTracker * tracker; + + + tracker = (struct _PurpleKeyringChangeTracker *)data; + new = tracker->new; + /* XXX check for read error or just forward ? */ - /* XXX change to use accessor */ - - //typedef void (*PurpleKeyringSave)(const PurpleAccount * account, gchar * password, GError ** error, PurpleKeyringSaveCallback cb, gpointer data); - + 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 @@ void purple_keyring_export_password(PurpleAccount * account, GError ** error, - PurpleKeyringImportCallback cb, + PurpleKeyringExportCallback cb, gpointer data) { PurpleKeyringExportPassword export; @@ -243,7 +307,7 @@ 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 * -purple_keyring_password_node_new(void) +purple_keyring_password_node_new() { PurpleKeyringPasswordNode * ret; @@ -487,7 +551,7 @@ } GQuark -purple_keyring_error_domain(void) +purple_keyring_error_domain() { return g_quark_from_static_string("Libpurple keyring"); }
--- a/libpurple/keyring.h Thu Jul 10 20:56:48 2008 +0000 +++ b/libpurple/keyring.h Fri Jul 11 22:26:24 2008 +0000 @@ -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 (*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 @@ /***************************************/ /* 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(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(PurpleAccount * account, GError ** error, - PurpleKeyringImportCallback cb, + PurpleKeyringExportCallback cb, gpointer data); @@ -170,7 +204,7 @@ /* 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 @@ /** @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_ */
--- a/libpurple/plugins/keyrings/internalkeyring.c Thu Jul 10 20:56:48 2008 +0000 +++ b/libpurple/plugins/keyrings/internalkeyring.c Fri Jul 11 22:26:24 2008 +0000 @@ -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 @@ /** * 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 @@ if (pwinfo->password == NULL) { + // FIXME : error cb(NULL, error, data); return;