Use hash table for bookmarks

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);

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) {

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;

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 void _bookmark_destroy(Bookmark *bookmark);
-static int _match_bookmark_by_jid(gconstpointer a, gconstpointer b);
 static void _send_bookmarks(void);
 
 void
 bookmark_request(void)
 {
-    char *id;
+    if (bookmarks) {
-    xmpp_ctx_t *ctx = connection_get_ctx();
+        g_hash_table_destroy(bookmarks);
-    xmpp_stanza_t *iq;
+    }
-
+    bookmarks = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)_bookmark_destroy);
-    id = strdup("bookmark_init_request");
 
     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));
+    assert(jid != NULL);
-    item->jid = strdup(jid);
-    item->nick = NULL;
-    item->password = NULL;
-    item->autojoin = FALSE;
 
-    GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid);
+    Bookmark *bookmark = g_hash_table_lookup(bookmarks, jid);
-    _bookmark_item_destroy(item);
+    if (!bookmark) {
-    if (found == NULL) {
         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));
+    assert(jid != NULL);
-    item->jid = strdup(jid);
-    item->nick = NULL;
-    item->password = NULL;
-    item->autojoin = FALSE;
 
-    GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid);
+    Bookmark *bookmark = g_hash_table_lookup(bookmarks, jid);
-    _bookmark_item_destroy(item);
+    if (!bookmark) {
-    if (found == NULL) {
         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));
+    assert(jid != NULL);
-    item->jid = strdup(jid);
-    item->nick = NULL;
-    item->password = NULL;
-    item->autojoin = FALSE;
 
-    GList *found = g_list_find_custom(bookmark_list, item, _match_bookmark_by_jid);
+    Bookmark *bookmark = g_hash_table_lookup(bookmarks, jid);
-    _bookmark_item_destroy(item);
+    if (!bookmark) {
-    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 {
         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);
+    return g_hash_table_contains(bookmarks, room);
-    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;
 }
 
 static int
 _bookmark_result_id_handler(xmpp_stanza_t *const stanza, void *const userdata)
 {
-    xmpp_ctx_t *ctx = connection_get_ctx();
+    const char *name = xmpp_stanza_get_name(stanza);
-    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);
     if (!name || strcmp(name, STANZA_NAME_IQ) != 0) {
         return 0;
     }
 
-    ptr = xmpp_stanza_get_child_by_name(stanza, STANZA_NAME_QUERY);
+    xmpp_stanza_t *query = xmpp_stanza_get_child_by_name(stanza, STANZA_NAME_QUERY);
-    if (!ptr) {
+    if (!query) {
         return 0;
     }
-    ptr = xmpp_stanza_get_child_by_name(ptr, STANZA_NAME_STORAGE);
+    xmpp_stanza_t *storage = xmpp_stanza_get_child_by_name(query, STANZA_NAME_STORAGE);
-    if (!ptr) {
+    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);
+    xmpp_stanza_t *child = xmpp_stanza_get_children(storage);
-    while (ptr) {
+    while (child) {
-        name = xmpp_stanza_get_name(ptr);
+        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;
+        char *nick = NULL;
-        nick_st = xmpp_stanza_get_child_by_name(ptr, "nick");
+        xmpp_stanza_t *nick_st = xmpp_stanza_get_child_by_name(child, "nick");
         if (nick_st) {
-            char *tmp;
+            nick = stanza_text_strdup(nick_st);
-            tmp = xmpp_stanza_get_text(nick_st);
-            if (tmp) {
-                nick = strdup(tmp);
-                xmpp_free(ctx, tmp);
-            }
         }
 
-        password = NULL;
+        char *password = NULL;
-        password_st = xmpp_stanza_get_child_by_name(ptr, "password");
+        xmpp_stanza_t *password_st = xmpp_stanza_get_child_by_name(child, "password");
         if (password_st) {
-            char *tmp;
+            password = stanza_text_strdup(password_st);
-            tmp = xmpp_stanza_get_text(password_st);
-            if (tmp) {
-                password = strdup(tmp);
-                xmpp_free(ctx, tmp);
-            }
         }
 
-        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));
+        Bookmark *bookmark = malloc(sizeof(Bookmark));
-        item->jid = strdup(jid);
+        bookmark->jid = strdup(jid);
-        item->nick = nick;
+        bookmark->nick = nick;
-        item->password = password;
+        bookmark->password = password;
-        item->autojoin = autojoin_val;
+        bookmark->autojoin = autojoin_val;
-        bookmark_list = g_list_append(bookmark_list, item);
+        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 (!bookmark) {
-
-    if (p == NULL) {
         return;
     }
 
-    free(p->jid);
+    free(bookmark->jid);
-    free(p->nick);
+    free(bookmark->nick);
-    free(p->password);
+    free(bookmark->password);
-    free(p);
+    free(bookmark);
-}
-
-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);
 }
 
 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);

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);
 }
 

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;
-}

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);
 

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);

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)

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();
 }