From 088606280626b8b4d7f303e54f69efd7d3bdb76a Mon Sep 17 00:00:00 2001 From: James Booth Date: Sun, 21 Aug 2016 15:25:49 +0100 Subject: [PATCH] Use hash table for bookmarks --- src/command/cmd_ac.c | 1 + src/command/cmd_funcs.c | 3 +- src/xmpp/blocking.c | 15 +- src/xmpp/bookmark.c | 327 +++++++++++----------------- src/xmpp/presence.c | 3 + src/xmpp/stanza.c | 43 ++-- src/xmpp/stanza.h | 2 + src/xmpp/xmpp.h | 2 +- tests/unittests/test_cmd_bookmark.c | 2 - tests/unittests/xmpp/stub_xmpp.c | 2 +- 10 files changed, 168 insertions(+), 232 deletions(-) diff --git a/src/command/cmd_ac.c b/src/command/cmd_ac.c index 6cb7b1a1..6bab07ce 100644 --- a/src/command/cmd_ac.c +++ b/src/command/cmd_ac.c @@ -1624,6 +1624,7 @@ _bookmark_autocomplete(ProfWin *window, const char *const input) return found; } } + g_string_free(beginning, TRUE); } g_strfreev(args); diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 192b4611..f163d3bc 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -4359,8 +4359,9 @@ cmd_bookmark(ProfWin *window, const char *const command, gchar **args) } return TRUE; } else if (strcmp(cmd, "list") == 0) { - const GList *bookmarks = bookmark_get_list(); + GList *bookmarks = bookmark_get_list(); cons_show_bookmarks(bookmarks); + g_list_free(bookmarks); } else { char *jid = args[1]; if (jid == NULL) { diff --git a/src/xmpp/blocking.c b/src/xmpp/blocking.c index f00bb429..88266068 100644 --- a/src/xmpp/blocking.c +++ b/src/xmpp/blocking.c @@ -63,10 +63,6 @@ static Autocomplete blocked_ac; void blocking_request(void) { - char *id = create_unique_id("blocked_list_request"); - xmpp_ctx_t *ctx = connection_get_ctx(); - xmpp_stanza_t *iq; - if (blocked) { g_list_free_full(blocked, free); blocked = NULL; @@ -77,13 +73,16 @@ blocking_request(void) } blocked_ac = autocomplete_new(); + char *id = create_unique_id("blocked_list_request"); iq_id_handler_add(id, _blocklist_result_handler, NULL, NULL); - iq = stanza_create_blocked_list_request(ctx); + xmpp_ctx_t *ctx = connection_get_ctx(); + xmpp_stanza_t *iq = stanza_create_blocked_list_request(ctx); xmpp_stanza_set_id(iq, id); + free(id); + iq_send_stanza(iq); xmpp_stanza_release(iq); - free(id); } GList* @@ -234,7 +233,7 @@ _block_add_result_handler(xmpp_stanza_t *const stanza, void *const userdata) char *jid = (char*)userdata; const char *type = xmpp_stanza_get_type(stanza); - if (type == NULL) { + if (!type) { log_info("Block response received for %s with no type attribute.", jid); free(jid); return 0; @@ -258,7 +257,7 @@ _block_remove_result_handler(xmpp_stanza_t *const stanza, void *const userdata) char *jid = (char*)userdata; const char *type = xmpp_stanza_get_type(stanza); - if (type == NULL) { + if (!type) { log_info("Unblock response received for %s with no type attribute.", jid); free(jid); return 0; diff --git a/src/xmpp/bookmark.c b/src/xmpp/bookmark.c index bc48ee33..8b9b6758 100644 --- a/src/xmpp/bookmark.c +++ b/src/xmpp/bookmark.c @@ -63,35 +63,32 @@ #define BOOKMARK_TIMEOUT 5000 static Autocomplete bookmark_ac; -static GList *bookmark_list; +static GHashTable *bookmarks; // id handlers static int _bookmark_result_id_handler(xmpp_stanza_t *const stanza, void *const userdata); -static void _bookmark_item_destroy(gpointer item); -static int _match_bookmark_by_jid(gconstpointer a, gconstpointer b); +static void _bookmark_destroy(Bookmark *bookmark); static void _send_bookmarks(void); void bookmark_request(void) { - char *id; - xmpp_ctx_t *ctx = connection_get_ctx(); - xmpp_stanza_t *iq; - - id = strdup("bookmark_init_request"); + if (bookmarks) { + g_hash_table_destroy(bookmarks); + } + bookmarks = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)_bookmark_destroy); autocomplete_free(bookmark_ac); bookmark_ac = autocomplete_new(); - if (bookmark_list) { - g_list_free_full(bookmark_list, _bookmark_item_destroy); - bookmark_list = NULL; - } - iq_id_handler_add(id, _bookmark_result_id_handler, free, id); + char *id = "bookmark_init_request"; + iq_id_handler_add(id, _bookmark_result_id_handler, free, NULL); - iq = stanza_create_bookmarks_storage_request(ctx); + xmpp_ctx_t *ctx = connection_get_ctx(); + xmpp_stanza_t *iq = stanza_create_bookmarks_storage_request(ctx); xmpp_stanza_set_id(iq, id); + iq_send_stanza(iq); xmpp_stanza_release(iq); } @@ -99,133 +96,119 @@ bookmark_request(void) gboolean bookmark_add(const char *jid, const char *nick, const char *password, const char *autojoin_str) { - if (autocomplete_contains(bookmark_ac, jid)) { + assert(jid != NULL); + + if (g_hash_table_contains(bookmarks, jid)) { return FALSE; - } else { - Bookmark *item = malloc(sizeof(*item)); - item->jid = strdup(jid); - if (nick) { - item->nick = strdup(nick); - } else { - item->nick = NULL; - } - if (password) { - item->password = strdup(password); - } else { - item->password = NULL; - } - - if (g_strcmp0(autojoin_str, "on") == 0) { - item->autojoin = TRUE; - } else { - item->autojoin = FALSE; - } - - bookmark_list = g_list_append(bookmark_list, item); - autocomplete_add(bookmark_ac, jid); - _send_bookmarks(); - - return TRUE; } + + Bookmark *bookmark = malloc(sizeof(Bookmark)); + bookmark->jid = strdup(jid); + if (nick) { + bookmark->nick = strdup(nick); + } else { + bookmark->nick = NULL; + } + if (password) { + bookmark->password = strdup(password); + } else { + bookmark->password = NULL; + } + + if (g_strcmp0(autojoin_str, "on") == 0) { + bookmark->autojoin = TRUE; + } else { + bookmark->autojoin = FALSE; + } + + g_hash_table_insert(bookmarks, strdup(jid), bookmark); + autocomplete_add(bookmark_ac, jid); + + _send_bookmarks(); + + return TRUE; } gboolean bookmark_update(const char *jid, const char *nick, const char *password, const char *autojoin_str) { - Bookmark *item = malloc(sizeof(*item)); - item->jid = strdup(jid); - item->nick = NULL; - item->password = NULL; - item->autojoin = FALSE; + assert(jid != NULL); - GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid); - _bookmark_item_destroy(item); - if (found == NULL) { + Bookmark *bookmark = g_hash_table_lookup(bookmarks, jid); + if (!bookmark) { return FALSE; - } else { - Bookmark *bm = found->data; - if (nick) { - free(bm->nick); - bm->nick = strdup(nick); - } - if (password) { - free(bm->password); - bm->password = strdup(password); - } - if (autojoin_str) { - if (g_strcmp0(autojoin_str, "on") == 0) { - bm->autojoin = TRUE; - } else if (g_strcmp0(autojoin_str, "off") == 0) { - bm->autojoin = FALSE; - } - } - _send_bookmarks(); - return TRUE; } + + if (nick) { + free(bookmark->nick); + bookmark->nick = strdup(nick); + } + if (password) { + free(bookmark->password); + bookmark->password = strdup(password); + } + if (autojoin_str) { + if (g_strcmp0(autojoin_str, "on") == 0) { + bookmark->autojoin = TRUE; + } else if (g_strcmp0(autojoin_str, "off") == 0) { + bookmark->autojoin = FALSE; + } + } + + _send_bookmarks(); + return TRUE; } gboolean bookmark_join(const char *jid) { - Bookmark *item = malloc(sizeof(*item)); - item->jid = strdup(jid); - item->nick = NULL; - item->password = NULL; - item->autojoin = FALSE; + assert(jid != NULL); - GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid); - _bookmark_item_destroy(item); - if (found == NULL) { + Bookmark *bookmark = g_hash_table_lookup(bookmarks, jid); + if (!bookmark) { return FALSE; - } else { - char *account_name = session_get_account_name(); - ProfAccount *account = accounts_get_account(account_name); - Bookmark *item = found->data; - if (!muc_active(item->jid)) { - char *nick = item->nick; - if (nick == NULL) { - nick = account->muc_nick; - } - presence_join_room(item->jid, nick, item->password); - muc_join(item->jid, nick, item->password, FALSE); - account_free(account); - } else if (muc_roster_complete(item->jid)) { - ui_room_join(item->jid, TRUE); - account_free(account); - } - return TRUE; } + + char *account_name = session_get_account_name(); + ProfAccount *account = accounts_get_account(account_name); + if (!muc_active(bookmark->jid)) { + char *nick = bookmark->nick; + if (!nick) { + nick = account->muc_nick; + } + presence_join_room(bookmark->jid, nick, bookmark->password); + muc_join(bookmark->jid, nick, bookmark->password, FALSE); + account_free(account); + } else if (muc_roster_complete(bookmark->jid)) { + ui_room_join(bookmark->jid, TRUE); + account_free(account); + } + + return TRUE; } gboolean bookmark_remove(const char *jid) { - Bookmark *item = malloc(sizeof(*item)); - item->jid = strdup(jid); - item->nick = NULL; - item->password = NULL; - item->autojoin = FALSE; + assert(jid != NULL); - GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid); - _bookmark_item_destroy(item); - gboolean removed = found != NULL; - - if (removed) { - bookmark_list = g_list_remove_link(bookmark_list, found); - _bookmark_item_destroy(found->data); - g_list_free(found); - autocomplete_remove(bookmark_ac, jid); - _send_bookmarks(); - return TRUE; - } else { + Bookmark *bookmark = g_hash_table_lookup(bookmarks, jid); + if (!bookmark) { return FALSE; } + + g_hash_table_remove(bookmarks, jid); + autocomplete_remove(bookmark_ac, jid); + + _send_bookmarks(); + + return TRUE; } -const GList* +GList* bookmark_get_list(void) { - return bookmark_list; + return g_hash_table_get_values(bookmarks); } char* @@ -245,110 +228,70 @@ bookmark_autocomplete_reset(void) gboolean bookmark_exists(const char *const room) { - GSList *bookmarks = autocomplete_create_list(bookmark_ac); - GSList *curr = bookmarks; - while (curr) { - if (strcmp(curr->data, room) == 0) { - g_slist_free_full(bookmarks, g_free); - return TRUE; - } else { - curr = g_slist_next(curr); - } - } - g_slist_free_full(bookmarks, g_free); - - return FALSE; + return g_hash_table_contains(bookmarks, room); } static int _bookmark_result_id_handler(xmpp_stanza_t *const stanza, void *const userdata) { - xmpp_ctx_t *ctx = connection_get_ctx(); - char *id = (char *)userdata; - xmpp_stanza_t *ptr; - xmpp_stanza_t *nick_st; - xmpp_stanza_t *password_st; - const char *name; - const char *jid; - const char *autojoin; - char *nick; - char *password; - gboolean autojoin_val; - Jid *my_jid; - Bookmark *item; - - g_free(id); - - name = xmpp_stanza_get_name(stanza); + const char *name = xmpp_stanza_get_name(stanza); if (!name || strcmp(name, STANZA_NAME_IQ) != 0) { return 0; } - ptr = xmpp_stanza_get_child_by_name(stanza, STANZA_NAME_QUERY); - if (!ptr) { + xmpp_stanza_t *query = xmpp_stanza_get_child_by_name(stanza, STANZA_NAME_QUERY); + if (!query) { return 0; } - ptr = xmpp_stanza_get_child_by_name(ptr, STANZA_NAME_STORAGE); - if (!ptr) { + xmpp_stanza_t *storage = xmpp_stanza_get_child_by_name(query, STANZA_NAME_STORAGE); + if (!storage) { return 0; } if (bookmark_ac == NULL) { bookmark_ac = autocomplete_new(); } - my_jid = jid_create(connection_get_fulljid()); - ptr = xmpp_stanza_get_children(ptr); - while (ptr) { - name = xmpp_stanza_get_name(ptr); + xmpp_stanza_t *child = xmpp_stanza_get_children(storage); + while (child) { + name = xmpp_stanza_get_name(child); if (!name || strcmp(name, STANZA_NAME_CONFERENCE) != 0) { - ptr = xmpp_stanza_get_next(ptr); + child = xmpp_stanza_get_next(child); continue; } - jid = xmpp_stanza_get_attribute(ptr, STANZA_ATTR_JID); + const char *jid = xmpp_stanza_get_attribute(child, STANZA_ATTR_JID); if (!jid) { - ptr = xmpp_stanza_get_next(ptr); + child = xmpp_stanza_get_next(child); continue; } log_debug("Handle bookmark for %s", jid); - nick = NULL; - nick_st = xmpp_stanza_get_child_by_name(ptr, "nick"); + char *nick = NULL; + xmpp_stanza_t *nick_st = xmpp_stanza_get_child_by_name(child, "nick"); if (nick_st) { - char *tmp; - tmp = xmpp_stanza_get_text(nick_st); - if (tmp) { - nick = strdup(tmp); - xmpp_free(ctx, tmp); - } + nick = stanza_text_strdup(nick_st); } - password = NULL; - password_st = xmpp_stanza_get_child_by_name(ptr, "password"); + char *password = NULL; + xmpp_stanza_t *password_st = xmpp_stanza_get_child_by_name(child, "password"); if (password_st) { - char *tmp; - tmp = xmpp_stanza_get_text(password_st); - if (tmp) { - password = strdup(tmp); - xmpp_free(ctx, tmp); - } + password = stanza_text_strdup(password_st); } - autojoin = xmpp_stanza_get_attribute(ptr, "autojoin"); + const char *autojoin = xmpp_stanza_get_attribute(child, "autojoin"); + gboolean autojoin_val = FALSE;; if (autojoin && (strcmp(autojoin, "1") == 0 || strcmp(autojoin, "true") == 0)) { autojoin_val = TRUE; - } else { - autojoin_val = FALSE; } autocomplete_add(bookmark_ac, jid); - item = malloc(sizeof(*item)); - item->jid = strdup(jid); - item->nick = nick; - item->password = password; - item->autojoin = autojoin_val; - bookmark_list = g_list_append(bookmark_list, item); + Bookmark *bookmark = malloc(sizeof(Bookmark)); + bookmark->jid = strdup(jid); + bookmark->nick = nick; + bookmark->password = password; + bookmark->autojoin = autojoin_val; + g_hash_table_insert(bookmarks, strdup(jid), bookmark); if (autojoin_val) { Jid *room_jid; @@ -369,36 +312,23 @@ _bookmark_result_id_handler(xmpp_stanza_t *const stanza, void *const userdata) account_free(account); } - ptr = xmpp_stanza_get_next(ptr); + child = xmpp_stanza_get_next(child); } - jid_destroy(my_jid); - return 0; } static void -_bookmark_item_destroy(gpointer item) +_bookmark_destroy(Bookmark *bookmark) { - Bookmark *p = (Bookmark *)item; - - if (p == NULL) { + if (!bookmark) { return; } - free(p->jid); - free(p->nick); - free(p->password); - free(p); -} - -static int -_match_bookmark_by_jid(gconstpointer a, gconstpointer b) -{ - Bookmark *bookmark_a = (Bookmark *) a; - Bookmark *bookmark_b = (Bookmark *) b; - - return strcmp(bookmark_a->jid, bookmark_b->jid); + free(bookmark->jid); + free(bookmark->nick); + free(bookmark->password); + free(bookmark); } static void @@ -418,6 +348,7 @@ _send_bookmarks(void) xmpp_stanza_set_name(storage, STANZA_NAME_STORAGE); xmpp_stanza_set_ns(storage, "storage:bookmarks"); + GList *bookmark_list = g_hash_table_get_values(bookmarks); GList *curr = bookmark_list; while (curr) { Bookmark *bookmark = curr->data; @@ -467,6 +398,8 @@ _send_bookmarks(void) curr = curr->next; } + g_list_free(bookmark_list); + xmpp_stanza_add_child(query, storage); xmpp_stanza_add_child(iq, query); xmpp_stanza_release(storage); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index 6b13f4a7..4c686b7d 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -780,6 +780,8 @@ _muc_user_self_handler(xmpp_stanza_t *stanza) free(status_str); free(reason); } + + jid_destroy(from_jid); } static void @@ -858,6 +860,7 @@ _muc_user_occupant_handler(xmpp_stanza_t *stanza) free(reason); } + jid_destroy(from_jid); free(status_str); } diff --git a/src/xmpp/stanza.c b/src/xmpp/stanza.c index bf4e44e1..ed7d0fce 100644 --- a/src/xmpp/stanza.c +++ b/src/xmpp/stanza.c @@ -66,7 +66,6 @@ #include "xmpp/muc.h" static void _stanza_add_unique_id(xmpp_stanza_t *stanza, char *prefix); -static char* _stanza_text_to_str(xmpp_stanza_t *stanza); #if 0 xmpp_stanza_t* @@ -1193,7 +1192,7 @@ stanza_get_status(xmpp_stanza_t *stanza, char *def) xmpp_stanza_t *status = xmpp_stanza_get_child_by_name(stanza, STANZA_NAME_STATUS); if (status) { - return _stanza_text_to_str(status); + return stanza_text_strdup(status); } else if (def) { return strdup(def); } else { @@ -1207,7 +1206,7 @@ stanza_get_show(xmpp_stanza_t *stanza, char *def) xmpp_stanza_t *show = xmpp_stanza_get_child_by_name(stanza, STANZA_NAME_SHOW); if (show) { - return _stanza_text_to_str(show); + return stanza_text_strdup(show); } else if (def) { return strdup(def); } else { @@ -1438,7 +1437,7 @@ stanza_get_muc_destroy_alternative_password(xmpp_stanza_t *stanza) return NULL; } - return _stanza_text_to_str(password_st); + return stanza_text_strdup(password_st); } char* @@ -1464,7 +1463,7 @@ stanza_get_muc_destroy_reason(xmpp_stanza_t *stanza) return NULL; } - return _stanza_text_to_str(reason_st); + return stanza_text_strdup(reason_st); } const char* @@ -1526,7 +1525,7 @@ stanza_get_reason(xmpp_stanza_t *stanza) return NULL; } - return _stanza_text_to_str(reason_st); + return stanza_text_strdup(reason_st); } gboolean @@ -1765,7 +1764,7 @@ stanza_get_error_message(xmpp_stanza_t *stanza) // check for text if (text_stanza) { - char *err_msg = _stanza_text_to_str(text_stanza); + char *err_msg = stanza_text_strdup(text_stanza); if (err_msg) { return err_msg; } @@ -1933,6 +1932,21 @@ stanza_resource_from_presence(XMPPPresence *presence) return resource; } +char* +stanza_text_strdup(xmpp_stanza_t *stanza) +{ + xmpp_ctx_t *ctx = connection_get_ctx(); + + char *string = NULL; + char *stanza_text = xmpp_stanza_get_text(stanza); + if (stanza_text) { + string = strdup(stanza_text); + xmpp_free(ctx, stanza_text); + } + + return string; +} + void stanza_free_caps(XMPPCaps *caps) { @@ -2025,18 +2039,3 @@ _stanza_add_unique_id(xmpp_stanza_t *stanza, char *prefix) xmpp_stanza_set_id(stanza, id); free(id); } - -static char* -_stanza_text_to_str(xmpp_stanza_t *stanza) -{ - xmpp_ctx_t *ctx = connection_get_ctx(); - - char *string = NULL; - char *stanza_text = xmpp_stanza_get_text(stanza); - if (stanza_text) { - string = strdup(stanza_text); - xmpp_free(ctx, stanza_text); - } - - return string; -} diff --git a/src/xmpp/stanza.h b/src/xmpp/stanza.h index 5bc1ff74..2c923657 100644 --- a/src/xmpp/stanza.h +++ b/src/xmpp/stanza.h @@ -315,6 +315,8 @@ Resource* stanza_resource_from_presence(XMPPPresence *presence); XMPPPresence* stanza_parse_presence(xmpp_stanza_t *stanza, int *err); void stanza_free_presence(XMPPPresence *presence); +char* stanza_text_strdup(xmpp_stanza_t *stanza); + XMPPCaps* stanza_parse_caps(xmpp_stanza_t *const stanza); void stanza_free_caps(XMPPCaps *caps); diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h index 9c0690a2..6157b648 100644 --- a/src/xmpp/xmpp.h +++ b/src/xmpp/xmpp.h @@ -190,7 +190,7 @@ gboolean bookmark_add(const char *jid, const char *nick, const char *password, c gboolean bookmark_update(const char *jid, const char *nick, const char *password, const char *autojoin_str); gboolean bookmark_remove(const char *jid); gboolean bookmark_join(const char *jid); -const GList* bookmark_get_list(void); +GList* bookmark_get_list(void); char* bookmark_find(const char *const search_str); void bookmark_autocomplete_reset(void); gboolean bookmark_exists(const char *const room); diff --git a/tests/unittests/test_cmd_bookmark.c b/tests/unittests/test_cmd_bookmark.c index f7a2cc4b..759b8238 100644 --- a/tests/unittests/test_cmd_bookmark.c +++ b/tests/unittests/test_cmd_bookmark.c @@ -126,8 +126,6 @@ void cmd_bookmark_list_shows_bookmarks(void **state) gboolean result = cmd_bookmark(&window, CMD_BOOKMARK, args); assert_true(result); - - g_list_free_full(bookmarks, (GDestroyNotify)_free_bookmark); } void cmd_bookmark_add_shows_message_when_invalid_jid(void **state) diff --git a/tests/unittests/xmpp/stub_xmpp.c b/tests/unittests/xmpp/stub_xmpp.c index c474e82a..e4ef7ea1 100644 --- a/tests/unittests/xmpp/stub_xmpp.c +++ b/tests/unittests/xmpp/stub_xmpp.c @@ -244,7 +244,7 @@ gboolean bookmark_join(const char *jid) return FALSE; } -const GList * bookmark_get_list(void) +GList * bookmark_get_list(void) { return (GList *)mock(); }