pidgin/pidgin

Avoid manual handling of icons in request field list items

2021-04-05, Arkadiy Illarionov
b5eccef8404e
Parents 1ec9131142c3
Children 73407e00ec5d
Avoid manual handling of icons in request field list items

* Use `PurpleKeyValuePair` as field list items
* Replace `icons` list with `has_icons` flag, thus fixing memleak
* Replace `purple_request_field_list_get_icons` with `purple_request_field_list_has_icons`

Testing Done:
Compile and run.

Reviewed at https://reviews.imfreedom.org/r/563/
6 files changed, 69 insertions(+), 82 deletions(-)
  • +3 -0
    ChangeLog.API
  • +20 -20
    finch/gntrequest.c
  • +4 -2
    libpurple/protocols/silc/chat.c
  • +13 -30
    libpurple/request.c
  • +6 -7
    libpurple/request.h
  • +23 -23
    pidgin/gtkrequest.c
    • --- a/ChangeLog.API Mon Mar 29 23:16:56 2021 -0500
    +++ b/ChangeLog.API Mon Apr 05 20:20:33 2021 -0500
    @@ -51,6 +51,7 @@
    * purple_request_field_choice_add_full
    * purple_request_field_get_tooltip
    * purple_request_field_group_get_fields_list
    + * purple_request_field_list_has_icons
    * purple_request_field_set_tooltip
    * purple_request_fields_get_ui_data
    * purple_request_fields_set_ui_data
    @@ -226,6 +227,7 @@
    purple_request_field_get_field_type
    * PurpleRoomlist is now a GObject. Please see the documentation for
    details.
    + * purple_request_field_list_get_items now returns list of PurpleKeyValuePair
    * purple_roomlist_room_get_type is now
    purple_roomlist_room_get_room_type
    * purple_roomlist_field_get_type is now
    @@ -496,6 +498,7 @@
    * purple_request_field_choice_set_data_destructor. Use
    purple_request_field_choice_add_full instead
    * purple_request_field_list_add
    + * purple_request_field_list_get_icons
    * PurpleSetPublicAliasFailureCallback
    * PurpleSetPublicAliasSuccessCallback
    * purple_socket_get_family
    --- a/finch/gntrequest.c Mon Mar 29 23:16:56 2021 -0500
    +++ b/finch/gntrequest.c Mon Apr 05 20:20:33 2021 -0500
    @@ -328,18 +328,20 @@
    }
    else if (type == PURPLE_REQUEST_FIELD_LIST)
    {
    - GList *list = NULL, *iter;
    + GList *selected = NULL;
    + GList *list = purple_request_field_list_get_items(field);
    if (purple_request_field_list_get_multi_select(field))
    {
    GntWidget *tree = purple_request_field_get_ui_data(field);
    - iter = purple_request_field_list_get_items(field);
    - for (; iter; iter = iter->next)
    + for (; list; list = list->next)
    {
    - const char *text = iter->data;
    + PurpleKeyValuePair *item = list->data;
    + const char *text = item->key;
    gpointer key = purple_request_field_list_get_data(field, text);
    - if (gnt_tree_get_choice(GNT_TREE(tree), key))
    - list = g_list_prepend(list, (gpointer)text);
    + if (gnt_tree_get_choice(GNT_TREE(tree), key)) {
    + selected = g_list_prepend(selected, (gpointer)text);
    + }
    }
    }
    else
    @@ -347,19 +349,19 @@
    GntWidget *combo = purple_request_field_get_ui_data(field);
    gpointer data = gnt_combo_box_get_selected_data(GNT_COMBO_BOX(combo));
    - iter = purple_request_field_list_get_items(field);
    - for (; iter; iter = iter->next) {
    - const char *text = iter->data;
    + for (; list; list = list->next) {
    + PurpleKeyValuePair *item = list->data;
    + const char *text = item->key;
    gpointer key = purple_request_field_list_get_data(field, text);
    if (key == data) {
    - list = g_list_prepend(list, (gpointer)text);
    + selected = g_list_prepend(selected, (gpointer)text);
    break;
    }
    }
    }
    - purple_request_field_list_set_selected(field, list);
    - g_list_free(list);
    + purple_request_field_list_set_selected(field, selected);
    + g_list_free(selected);
    }
    else if (type == PURPLE_REQUEST_FIELD_ACCOUNT)
    {
    @@ -478,16 +480,14 @@
    create_list_field(PurpleRequestField *field)
    {
    GntWidget *ret = NULL;
    - GList *list;
    - gboolean multi = purple_request_field_list_get_multi_select(field);
    - if (multi)
    - {
    + GList *list = purple_request_field_list_get_items(field);
    + if (purple_request_field_list_get_multi_select(field)) {
    GntWidget *tree = gnt_tree_new();
    - list = purple_request_field_list_get_items(field);
    for (; list; list = list->next)
    {
    - const char *text = list->data;
    + PurpleKeyValuePair *item = list->data;
    + const char *text = item->key;
    gpointer key = purple_request_field_list_get_data(field, text);
    gnt_tree_add_choice(GNT_TREE(tree), key,
    gnt_tree_create_row(GNT_TREE(tree), text), NULL, NULL);
    @@ -500,10 +500,10 @@
    {
    GntWidget *combo = gnt_combo_box_new();
    - list = purple_request_field_list_get_items(field);
    for (; list; list = list->next)
    {
    - const char *text = list->data;
    + PurpleKeyValuePair *item = list->data;
    + const char *text = item->key;
    gpointer key = purple_request_field_list_get_data(field, text);
    gnt_combo_box_add_data(GNT_COMBO_BOX(combo), key, text);
    if (purple_request_field_list_is_selected(field, text))
    --- a/libpurple/protocols/silc/chat.c Mon Mar 29 23:16:56 2021 -0500
    +++ b/libpurple/protocols/silc/chat.c Mon Apr 05 20:20:33 2021 -0500
    @@ -295,8 +295,10 @@
    chpks = silc_buffer_alloc_size(2);
    for (ct = 0; list; list = list->next, ct++) {
    - public_key = purple_request_field_list_get_data(f, list->data);
    - if (purple_request_field_list_is_selected(f, list->data)) {
    + PurpleKeyValuePair *item = list->data;
    +
    + public_key = purple_request_field_list_get_data(f, item->key);
    + if (purple_request_field_list_is_selected(f, item->key)) {
    /* Delete this public key */
    pk = silc_public_key_payload_encode(public_key);
    chpks = silc_argument_payload_encode_one(chpks, pk->data,
    --- a/libpurple/request.c Mon Mar 29 23:16:56 2021 -0500
    +++ b/libpurple/request.c Mon Apr 05 20:20:33 2021 -0500
    @@ -98,7 +98,7 @@
    struct
    {
    GList *items;
    - GList *icons;
    + gboolean has_icons;
    GHashTable *item_data;
    GList *selected;
    GHashTable *selected_table;
    @@ -947,7 +947,7 @@
    }
    else if (field->type == PURPLE_REQUEST_FIELD_LIST)
    {
    - g_list_free_full(field->u.list.items, g_free);
    + g_list_free_full(field->u.list.items, (GDestroyNotify)purple_key_value_pair_free);
    g_list_free_full(field->u.list.selected, g_free);
    g_hash_table_destroy(field->u.list.item_data);
    g_hash_table_destroy(field->u.list.selected_table);
    @@ -1578,33 +1578,16 @@
    purple_request_field_list_add_icon(PurpleRequestField *field, const char *item, const char* icon_path,
    void *data)
    {
    + PurpleKeyValuePair *kvp;
    +
    g_return_if_fail(field != NULL);
    g_return_if_fail(item != NULL);
    g_return_if_fail(data != NULL);
    g_return_if_fail(field->type == PURPLE_REQUEST_FIELD_LIST);
    - if (icon_path)
    - {
    - if (field->u.list.icons == NULL)
    - {
    - GList *l;
    - for (l = field->u.list.items ; l != NULL ; l = l->next)
    - {
    - /* Order doesn't matter, because we're just
    - * filing in blank items. So, we use
    - * g_list_prepend() because it's faster. */
    - field->u.list.icons = g_list_prepend(field->u.list.icons, NULL);
    - }
    - }
    - field->u.list.icons = g_list_append(field->u.list.icons, g_strdup(icon_path));
    - }
    - else if (field->u.list.icons)
    - {
    - /* Keep this even with the items list. */
    - field->u.list.icons = g_list_append(field->u.list.icons, NULL);
    - }
    -
    - field->u.list.items = g_list_append(field->u.list.items, g_strdup(item));
    + field->u.list.has_icons = field->u.list.has_icons || (icon_path != NULL);
    + kvp = purple_key_value_pair_new_full(item, g_strdup(icon_path), g_free);
    + field->u.list.items = g_list_append(field->u.list.items, kvp);
    g_hash_table_insert(field->u.list.item_data, g_strdup(item), data);
    }
    @@ -1707,13 +1690,13 @@
    return field->u.list.items;
    }
    -GList *
    -purple_request_field_list_get_icons(const PurpleRequestField *field)
    +gboolean
    +purple_request_field_list_has_icons(const PurpleRequestField *field)
    {
    - g_return_val_if_fail(field != NULL, NULL);
    - g_return_val_if_fail(field->type == PURPLE_REQUEST_FIELD_LIST, NULL);
    -
    - return field->u.list.icons;
    + g_return_val_if_fail(field != NULL, FALSE);
    + g_return_val_if_fail(field->type == PURPLE_REQUEST_FIELD_LIST, FALSE);
    +
    + return field->u.list.has_icons;
    }
    PurpleRequestField *
    --- a/libpurple/request.h Mon Mar 29 23:16:56 2021 -0500
    +++ b/libpurple/request.h Mon Apr 05 20:20:33 2021 -0500
    @@ -1622,22 +1622,21 @@
    *
    * Returns a list of items in a list field.
    *
    - * Returns: (element-type utf8) (transfer none): The list of items.
    + * Returns: (element-type PurpleKeyValuePair) (transfer none): The list of items.
    */
    GList *purple_request_field_list_get_items(const PurpleRequestField *field);
    /**
    - * purple_request_field_list_get_icons:
    + * purple_request_field_list_has_icons:
    * @field: The field.
    *
    - * Returns a list of icons in a list field.
    + * Indicates if list field has icons.
    *
    - * The icons will correspond with the items, in order.
    + * Returns: TRUE if list field has icons, FALSE otherwise.
    *
    - * Returns: (element-type utf8) (transfer none): The list of icons or %NULL (i.e. the empty #GList)
    - * if no items have icons.
    + * Since: 3.0.0
    */
    -GList *purple_request_field_list_get_icons(const PurpleRequestField *field);
    +gboolean purple_request_field_list_has_icons(const PurpleRequestField *field);
    /**************************************************************************/
    /* Label Field API */
    --- a/pidgin/gtkrequest.c Mon Mar 29 23:16:56 2021 -0500
    +++ b/pidgin/gtkrequest.c Mon Apr 05 20:20:33 2021 -0500
    @@ -1405,16 +1405,17 @@
    GtkTreeViewColumn *column;
    GtkTreeIter iter;
    GList *l;
    - GList *icons = NULL;
    -
    - icons = purple_request_field_list_get_icons(field);
    + gboolean has_icons;
    +
    + has_icons = purple_request_field_list_has_icons(field);
    /* Create the list store */
    - if (icons)
    + if (has_icons) {
    store = gtk_list_store_new(3, G_TYPE_POINTER, G_TYPE_STRING, GDK_TYPE_PIXBUF);
    - else
    + } else {
    store = gtk_list_store_new(2, G_TYPE_POINTER, G_TYPE_STRING);
    + }
    /* Create the tree view */
    treeview = gtk_tree_view_new_with_model(GTK_TREE_MODEL(store));
    @@ -1433,8 +1434,7 @@
    gtk_tree_view_column_pack_start(column, renderer, TRUE);
    gtk_tree_view_column_add_attribute(column, renderer, "text", 1);
    - if (icons)
    - {
    + if (has_icons) {
    renderer = gtk_cell_renderer_pixbuf_new();
    gtk_tree_view_column_pack_start(column, renderer, TRUE);
    gtk_tree_view_column_add_attribute(column, renderer, "pixbuf", 2);
    @@ -1444,33 +1444,33 @@
    for (l = purple_request_field_list_get_items(field); l != NULL; l = l->next)
    {
    - const char *text = (const char *)l->data;
    + PurpleKeyValuePair *item = l->data;
    + const char *text = (const char *)item->key;
    gtk_list_store_append(store, &iter);
    - if (icons)
    - {
    - const char *icon_path = (const char *)icons->data;
    + if (has_icons) {
    + const char *icon_path = (const char *)item->value;
    GdkPixbuf* pixbuf = NULL;
    if (icon_path)
    pixbuf = pidgin_pixbuf_new_from_file(icon_path);
    gtk_list_store_set(store, &iter,
    - 0, purple_request_field_list_get_data(field, text),
    - 1, text,
    - 2, pixbuf,
    - -1);
    - icons = icons->next;
    + 0, purple_request_field_list_get_data(field, text),
    + 1, text,
    + 2, pixbuf,
    + -1);
    + } else {
    + gtk_list_store_set(store, &iter,
    + 0, purple_request_field_list_get_data(field, text),
    + 1, text,
    + -1);
    }
    - else
    - gtk_list_store_set(store, &iter,
    - 0, purple_request_field_list_get_data(field, text),
    - 1, text,
    - -1);
    -
    - if (purple_request_field_list_is_selected(field, text))
    +
    + if (purple_request_field_list_is_selected(field, text)) {
    gtk_tree_selection_select_iter(sel, &iter);
    + }
    }
    /*