lightdash: Adding bookmarks buttons

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

lightdash: Adding bookmarks buttons

adlo
I am attempting to implement bookmarks buttons for lightdash:

https://github.com/adlocode/xfce4-lightdash-plugin/tree/bookmarks-buttons

However, it does not seem to work as intended. When adding items to bookmarks, it tends to create duplicate buttons. Also, it sometimes crashes when removing bookmarks.

What could be causing this, and how could I resolve it?

Regards

adlo
_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|

Re: lightdash: Adding bookmarks buttons

adlo
I am attempting to use XfceAppfinderModel's items GSList to get the items, and then query each item's is_bookmark property to determine if it should be a bookmark. However, this seems to result in duplicate buttons being added to the bar and can sometimes cause crashes.

How can I resolve this?

Regards

adlo



> On 16 Nov 2015, at 19:40, adlo <[hidden email]> wrote:
>
> I am attempting to implement bookmarks buttons for lightdash:
>
> https://github.com/adlocode/xfce4-lightdash-plugin/tree/bookmarks-buttons
>
> However, it does not seem to work as intended. When adding items to bookmarks, it tends to create duplicate buttons. Also, it sometimes crashes when removing bookmarks.
>
> What could be causing this, and how could I resolve it?
>
> Regards
>
> adlo
_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|

Re: lightdash: Adding bookmarks buttons

Matthew Brush
On 2015-11-19 10:21 AM, adlo wrote:
> I am attempting to use XfceAppfinderModel's items GSList to get the items, and then query each item's is_bookmark property to determine if it should be a bookmark. However, this seems to result in duplicate buttons being added to the bar and can sometimes cause crashes.
>
> How can I resolve this?
>

You should provide a link to the malfunctioning code so people can see
what you're doing (wrong). With the information you've given, it would
be really hard to help you, at least not without sifting through
thousands of lines of C code, guessing what lines you might be asking
about, assuming you even have the code pushed to your repo.

Cheers,
Matthew Brush

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|

Re: lightdash: Adding bookmarks buttons

adlo
> On 20 Nov 2015, at 02:49, Matthew Brush <[hidden email]> wrote:
>
> You should provide a link to the malfunctioning code so people can see what you're doing (wrong). With the information you've given, it would be really hard to help you, at least not without sifting through thousands of lines of C code, guessing what lines you might be asking about, assuming you even have the code pushed to your repo.

You may have a point there; I assumed that the content of my last commit to that branch would provide enough information.

Here is the code where the bookmarks-changed signal is emitted:
https://github.com/adlocode/xfce4-lightdash-plugin/blob/bookmarks-buttons/src/appfinder-model.c#L2461

Here is the code that handles bookmarks-changed events:
https://github.com/adlocode/xfce4-lightdash-plugin/blob/bookmarks-buttons/src/appfinder-window.c#L288

Does anyone have any ideas?

Regards

adlo

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|

Re: lightdash: Adding bookmarks buttons

Matthew Brush
On 2015-11-20 11:12 AM, adlo wrote:

>> On 20 Nov 2015, at 02:49, Matthew Brush <[hidden email]> wrote:
>>
>> You should provide a link to the malfunctioning code so people can see what you're doing (wrong). With the information you've given, it would be really hard to help you, at least not without sifting through thousands of lines of C code, guessing what lines you might be asking about, assuming you even have the code pushed to your repo.
>
> You may have a point there; I assumed that the content of my last commit to that branch would provide enough information.
>
> Here is the code where the bookmarks-changed signal is emitted:
> https://github.com/adlocode/xfce4-lightdash-plugin/blob/bookmarks-buttons/src/appfinder-model.c#L2461
>
> Here is the code that handles bookmarks-changed events:
> https://github.com/adlocode/xfce4-lightdash-plugin/blob/bookmarks-buttons/src/appfinder-window.c#L288
>
> Does anyone have any ideas?
>

At a quick glance, it might be because you're mutating the linked list
during traversal. Usually doing that requires a little finesse to ensure
you're not accessing dangling pointers.

One way for this[0] loop (untested):


     GList *li = window->bookmarks_buttons;
     while (li != NULL)
     {
         GList *next = li->next; // save next pointer
         GtkWidget *button = li->data;
         if (button != NULL) // conditionally mutate list
         {
             window->bookmarks_buttons =
                 g_list_delete_link(window->bookmarks_buttons, li);
             gtk_widget_destroy(button);
         }
         li = next; // move to next element
     }

Not sure that's 100% correct but it gives the idea. There's a number of
ways to code such a loop as well. For this specific case you could
probably also just do:

     g_list_free_full(window->bookmarks_buttons, gtk_widget_destroy);
     window->bookmarks_buttons = NULL;

Assuming you just want to delete everything in the list.

For the second loop[1], assuming `lightdash_model_get_items()` transfers
ownership of the list to the caller, you probably want to stash a
pointer to the head of that list, since you need to pass it to
`g_slist_free()` at line 323, as opposed to passing it `NULL` (what
`sli` will contain once that loop is finished).

Maybe like this:

     GSList *items = lightdash_get_items(model);
     for (GSList sli = items; sli != NULL; sli = sli->next)
     {
         // careful about mutating list here too !
     }
     g_slist_free(items);

One good tip for such types of bugs is to run the code in Valgrind
(though it's hard since GLib/GTK+ causes many "false positives" noise in
the output). It will tell you when you're accessing memory out of
bounds, double-freeing, accessing already freed memory, etc.

Cheers,
Matthew Brush

[0]:
https://github.com/adlocode/xfce4-lightdash-plugin/blob/bookmarks-buttons/src/appfinder-window.c#L297
[1]:
https://github.com/adlocode/xfce4-lightdash-plugin/blob/bookmarks-buttons/src/appfinder-window.c#L308
_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev