Time to Build

March 23, 2008 on 10:42 pm | In GNOME, recent-files, gtk | 3 Comments

Claudio did some interesting profiling (and patching) of the BookmarkFile implementation in GLib — so kudos to him and Felix.

one thing that he noted is:

However, I still have the feeling that letting ~\.recently-used.xbel grow without control is very, very wrong. In my laptop, this file is about 5MB, which accounts for ca. 9000 files(!).

this is very true, but I feel it needs some context. when I first wrote the RecentManager code I only had the EggRecent implementation as a comparison; the old EggRecentModel had an hardcoded limit of 500 items stored per file. limiting on the number, instead of the age of an item inside a recently used file list did not feel right, so I thought about hardcoding a limit of 30 days — but stopped short of doing it because I realized that hardcoding limits at the toolkit level was not a good idea:

  • application developers will not be able to change it in any way
  • users will not be able to change it in any way
  • system administrators will not be able to change it in any way

just to give a few examples: while I was still writing the RecentManager inside libegg, Alex Graveley was writing Gimmie. Gimmie had1 a local document and application history that could allow you to go back in time of months; had I hardcoded a limit, the Gimmie developers would have needed a new implementation, defeating the purpose of shipping the RecentManager inside GTK+ to cut down the amount of code replication.

hardcoding limits is also something that makes it hard, or even impossible, for users and administrators to control; I might want a 30 days limit, but other might want a 90 days, or a 7 days — or even a 1 day limit. some might not even want to save the recently used files at all (think kiosks).

I don’t believe in strictly hardcoding policies in the toolkit; providing fallbacks is perfectly fine, but preventing people from actually having different settings is akin to convince everyone that you’re right and they’re wrong.

still, this doesn’t solve the problem at hand, that is the current lack of policy.

what I’d like to see is some process taking care of purging the old entries, using some key inside gconf, at the end of the session; gnome-settings-daemon would fit the role for GNOME, and other desktop environments using GTK+ could provide the same functionality2. after all, gnome-settings-daemon should already flush the thumbnails cache — it wouldn’t be much of a complication.

  1. and might still have — I haven’t checked it for a while now []
  2. if you’re not using a GTK+ based desktop environment you’re either using the same spec used by GTK+ so you can provide your own way of purging the cache, or you’re using another way to store the recently used files, so the size of the file saved/read by the RecentManager will not bubble out of control so easily — and you can still flush it yourself []

Company Calls Epilogue

May 4, 2007 on 5:45 pm | In Hacking, GNOME, C, recent-files, gtk | 9 Comments

Today I gave the final touches to a patch based upon the patch for search capabilities in the GtkFileChooser embeddable widget, adding the “Recently Used” shortcut:

File Chooser Support for Recently Used Files

there are still a few missing bits (the icon for the shortcut is one of them) but it’s already working remarkably well. I’ll work toward implementing what’s left in the next few days, while I also fix some of the warts of the search support.

Time For Lovin’

March 15, 2007 on 10:15 pm | In Hacking, GNOME, recent-files, gtk | No Comments

You can see I’m listening to the Beastie Boys, right now

In the last three days I’ve finally managed to resolve as fixed these GTK+ bugs:

  • Bug 347375 – Not possible to set startup notification ID on a GtkWindow
  • Bug 418219 – GtkRecentChooser should apply filter before sorting and clamping the list
  • Bug 418634 – gtk_recent_chooser_menu_set_show_numbers docs
  • Bug 418673 – gtk_recent_manager_add_item
  • Bug 338843 – add recent files support inside the ui manager

Bug #347375 contained a patch Vytautas Liuolia wrote last year, as part of his guniqueapp Summer of Code project.

Bugs #418219, #418634 and #418673 have also been backported to the stable branch, as they fixed documentation or simply bad behaviours in the code.

Bug #338843 finally closes the long standing bug about the integration between the recent files and GtkUIManager; you can now add a GtkRecentAction to your actions group and let it do the right thing.

Many thanks go to Vytautas Liuolia, Matthias Clasen, Paolo Borelli and Morten Welinder.

Come and See

February 9, 2007 on 4:18 pm | In Hacking, GNOME, Life, recent-files, developer, fosdem, london, gtk, music | 1 Comment

Decemberists: Yesterday evening, The Decemberists were in London, and made a wonderful concert at Shepherd Bush. I’ve fallen in love with the band after vieweing the video of their song Sixteen Military Wives and I started getting my hands on their whole discography (right now, only the singles are missing). They played mostly songs from their new album, The Crane Wife, which is really as good as it can get (so go out and buy it now).

gnome-utils: It seems that my plea for someone to work on GFloppy has been useful; right now, Paul Betts and Riccardo Setti are working on a replacement using HAL and libparted, and are also getting the ball rolling for adding formatting support directly in HAL. Guys, you rock!

GTK+: I’ve been working on fixing some bugs of the GtkRecentChooserMenu widget; specifically, bug #377164 and bug #405696. While I was at it, I finally closed the FIXME I left in code, for supporting both appending and prepending custom menu items in the recent files menu, and finally added a test case for the widget, so I can track regressions.

Recent files menu

Obligatory screenshot of the test application

FOSDEM ‘07: Like last year, at the end of February I’ll be in Bruxelles, attending FOSDEM.

Boogie Woogie Bugle Boy

August 1, 2006 on 11:50 pm | In Hacking, C, recent-files, developer, gtk | 2 Comments

Lately there has been some activity in projects moving from EggRecent to GtkRecent. Obviously, this led to questions and bugs opened about the API.

One of the questions is: if I had a EggRecentModel singleton what should I use now?. Since EggRecent needed a EggRecentModel around for the entire lifetime of the recently used files list, you had to keep at least a singleton around; you also had to pass it to the objects (not widgets) displaying the list, because each istance of the objects would modify the list itself. GtkRecentManager does not need that, since the widgets usually use their own internal instance of the manager and they don’t change the list in any way. So you can create a new GtkRecentManager and keep it around as a singleton (remember to call g_object_unref() on it when finished, otherwise you’ll leak it):

GtkRecentManager *manager_singleton = gtk_recent_manager_new ();

and then pass it to the widgets:

GtkWidget *dialog =
  gtk_recent_chooser_dialog_new_with_manager (“Recent Documents”
                                              parent,
                                              manager_singleton,
                                              GTK_STOCK_CLOSE,  GTK_RESPONSE_CLOSE,
                                              GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
                                              NULL);

Nevertheless, you should really use gtk_recent_manager_get_default() which will do the right thing, and pass you a per-process instance and you won’t have to care about its memory management (no g_object_unref() when quitting). Also, using gtk_recent_manager_get_default() means that you don’t have to use the _with_manager variant of the widgets constructor, as they will use the per-process GtkRecentManager by default. Using your own GtkRecentManager makes sense only if you want to use the “filename” constructor-only property to specify your own storage file (and you should do that only if you know what you are doing):

GtkRecentManager *manager = gtk_recent_manager_get_default ();

  ...

GtkWidget *dialog =
  gtk_recent_chooser_dialog_new (“Recent Documents”
                                 parent,
                                 GTK_STOCK_CLOSE,  GTK_RESPONSE_CLOSE,
                                 GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
                                 NULL);

Another question is the dreaded I want an inline list of recent files in the File menu; I’ll redirect you to the bug report linked for the rationales about why I think the inline list sucks, and should be removed from the UIs of GNOME applications in favour of the sub-menu. Anyhow, for those Unbelievers still using this Windows-cloned relic of the past, here’s a simple function for adding an inline list to an existing GtkUIManager instance; it’s part of a example of integration between GtkRecent and the UI manager I sent to the gtk-devel mailing list before GUADEC.

static void
add_inline_recent_items (guint         merge_id,
                         GtkUIManager *ui_manager)
{
  GtkActionGroup *action_group;
  GtkRecentManager *manager;
  GList *items, *l;
  guint i;

  action_group = gtk_action_group_new (“recent-info”);
  gtk_ui_manager_insert_action_group (ui_manager, action_group, 0);

  manager = gtk_recent_manager_get_default ();

  /* we don’t care about sorting and filtering, but it can
   * be done using g_list_sort_with_data() and clamping the
   * resulting list to the desired length
   */
  gtk_recent_manager_set_limit (manager, 4);
  items = gtk_recent_manager_get_items (manager);

  /* no items: attach an insensitive place holder */
  if (!items)
    {
      GtkAction *action;

      action = g_object_new (GTK_TYPE_ACTION,
      			     “name”, “recent-info-empty”,
			     “label”, “No recently used files”,
			     “sensitive”, FALSE,
			     NULL);
      gtk_action_group_add_action (action_group, action);
      g_object_unref (action);

      gtk_ui_manager_add_ui (ui_manager, merge_id,
  			     “/MenuBar/FileMenu/RecentFiles”,
			     “recent-info-empty”,
			     “recent-info-empty”,
			     GTK_UI_MANAGER_MENUITEM,
			     FALSE);

      return;
    }

  for (i = 0, l = items;
       l != NULL;
       i += 1, l = l->next)
    {
      GtkRecentInfo *info = l->data;
      gchar *name = g_strdup_printf (“recent-info-%d-%lu”,
      				     i,
				     (gulong) time (NULL));
      gchar *action_name = g_strdup (name);
      GtkAction *action;

      action = g_object_new (GTK_TYPE_ACTION,
      			     “name”, action_name,
			     “label”, gtk_recent_info_get_display_name (info),
			     “stock_id”, NULL,
			     NULL);
      g_object_set_data_full (G_OBJECT (action), “gtk-recent-info”,
                              gtk_recent_info_ref (info),
			      (GDestroyNotify) gtk_recent_info_unref);
      g_signal_connect (action, “activate”,
                        G_CALLBACK (recent_activate_cb), NULL);
      gtk_action_group_add_action (action_group, action);
      g_object_unref (action);

      /* all you need is a UI definition with a “placeholder” element called
       * “RecentFiles” under a “menu” element called “FileMenu”.
       */
      gtk_ui_manager_add_ui (ui_manager, merge_id,
  			     “/MenuBar/FileMenu/RecentFiles”,
			     name,
			     action_name,
			     GTK_UI_MANAGER_MENUITEM,
			     FALSE);

      g_print (“* adding action `%s’n”, action_name);

      g_free (action_name);
      g_free (name);
    }

  /* unref the info objects so that they will be destroyed when
   * the actions to which they are bound are destroyed with the
   * UIManager instance
   */
  g_list_foreach (items, (GFunc) gtk_recent_info_unref, NULL);
  g_list_free (items);
}

This code should give you an idea on how to implement your own version. Making the menu reloading the inline list means removing the UI definitions using the merge_id integer and calling this function again inside a callback attached to the “changed” signal of a GtkRecentManager instance, and it is left as an exercise for the reader. ;-)

GtkRecent and GtkUIManager

I’m really sorry that the UIManager integration did not happen in time for 2.10; it means that some applications will have to implement it by themselves until GTK+ 2.12 is out with a common implementation. In the meantime, continue asking me or opening bugs in Bugzilla.

Update@2006-08-02T09:51+0100: the complete code now has the “reload-when-list-change” feature. Remember: you may cut and paste this code for a basic support of recent files inside an inline menu; there are at least another couple of ways to implement the same level of support, and mostly it depends on how you handle your own documents. I’d like for GTK+ or libgnome or GLib to have a “document” class abstracting most of this and other stuff, like Cocoa on OSX has the NSDocument class.

Update@2006-08-02T16:05+0100: another update - I’ve fixed a couple of leaks (a GtkAction is a GObject and not a GtkObject) and added the sorting function for the inlined list.

Tides that I tried to swim against

July 12, 2006 on 2:14 pm | In Hacking, Perl, C, recent-files, gtk, ohand, clutter | No Comments

gtk-recent: For those who missed the mail on gtk-devel-list, language-bindings and desktop-devel:

Unfortunately, when importing the GtkRecent API in GTK+ I made a mistake and these two functions have been erroneously left inside the GtkRecentChooser interface API:


  gtk_recent_chooser_set_show_numbers()
  gtk_recent_chooser_get_show_numbers()

These two functions try to set the “show-numbers” property, which is installed only by GtkRecentChooserMenu and it’s not one of the properties defined by the GtkRecentChooser interface. Using these functions on a GtkRecentChooserMenu (or any other custom GtkRecentChooser implementation which defines a boolean “show-numbers” property) will yield the expected results, while using them on a GtkRecentChooser implementation that does not support the “show-numbers” property will result in a warning.

Since we are in a stable release we can’t mark those functions as deprecated, and we cannot remove them without breaking the API. You are advised not to use these functions: use the GtkRecentChooserMenu functions instead:


  gtk_recent_chooser_menu_set_show_numbers()
  gtk_recent_chooser_menu_get_show_numbers()

Language binding authors should not bind those functions, but bind the GtkRecentChooserMenu functions instead.

These functions will be marked as deprecated as soon as GTK+ will branch off for the 2.11/2.12 cycle, so you’ll have to bear with this inconsistency for a short period of time.

language-bindings/1: By the way, gtk2-perl now supports the GtkRecent code in HEAD, thanks to the hard work of Torsten kaffee Schoenfeld who fixed most of my first iteration binding code and wrote the tests for it.

language-bindings/2: I also finished the Perl bindings for Clutter, as well as the Python ones. As I changed Clutter to behave like GTK, with the ClutterActor objects being created with a “floating” reference count, you’ll have to update Clutter to HEAD if you want to test the bindings. Beware that Clutter’s API is still a bit fuzzy at the moment. Of course, if you find bugs in the library or in the bindings, be sure to report them.

Now Listening: Last-exit, Neighbour radio

The Mariner’s Revenge Song

May 7, 2006 on 11:28 pm | In GNOME, recent-files, announce, gtk | No Comments

From the libegg’s ChangeLog:

2006-05-07  Emmanuele Bassi  

	Finally deprecate EggRecent.  So long, and thanks for all the bugs.

	* libegg/recent-files/THIS_IS_DEPRECATED_USE_GTK_RECENT_CHOOSER:
	* libegg/recent-files/egg-recent-model.h: Deprecate the EggRecent code,
	now that GTK 2.9.0 is out; if you want to compile it, you
	must define EGG_ENABLE_RECENT_FILES before including
	egg-recent-model.h.

This is the first version of the so-called “Ramone Deprecation System”: it means that if you blindly re-sync with libegg HEAD, or if you decide to use the EggRecent code now, a guy called Ramone will be sent directly to your home by the Gtk+ Cabal; he will politely knock at your door and once you’ve opened, he will beat the crap out of you.

Next, the Gtk+ Cabal will implement the “Puppies Deprecation System”. And believe me: you don’t want to know how that works.

Now listening: The Decemberists, Picaresque

For the price of a cup of tea

April 17, 2006 on 8:34 pm | In Hacking, GNOME, recent-files | 3 Comments

Here’s a screenshot of testaction test showing the GtkRecentAction action for use with the GtkUIManager. The action is bound to a menuitem tag inside the UI definition markup.

GtkRecentAction test

The code needs cleaning up and the hooking up of the GtkRecentChooser interface virtual methods, but it’s not a big issue; setting all the recent chooser knobs using the recent chooser API is not the most beautiful approach I could come up with, though. But so is setting all the properties using specially crafted functions.

The next issue is inlining the list inside a menu (to make the Gedit maintainers happy ;-) ). The current (and soon to be deprecated) EggRecent code used the awful EggRecentViewGtk object and its evil spawn (EggRecentViewUIManager and the most devilish EggRecentViewBonobo). The GtkRecent approach “get the list from the recent manager and build the menu items” works for hand-built menus, but for menus created using the UI manager there’s no real option, as there’s no way to create a list of items from a GtkAction. One way to implement it might be adding a GList * (*create_menu_items) (GtkAction *action); virtual method inside GtkActionClass.

Update 2006-04-18@03:51: the tracking bug with my initial draft for the GtkRecentAction is #338843

Now listening: Belle and Sebastian, The Life Pursuit

ChangeLog

April 17, 2006 on 12:25 pm | In Hacking, GNOME, Life, recent-files, gnome-utils | No Comments

moving out: Marta has been in London a couple of times, in these two weeks; she has found a nice house in Crystal Palace, and began to buy furniture and stuff, and preparing it for us to move in. Next week, we’re both going to London and buy the remaining stuff (a sofa-bed, a table for the living room and some chairs to begin with). We should return in Milan for a couple of days on the 4th of May, as we must sign documents and papers for the marriage, and actually choose its date (it’ll be in July, probably).

gtk+ recent: the merge went well; on the gtk-devel list, Murray Cumming pointed out a couple of issues; most notably, the lack of integration with the GtkUIManager. The plan was to add a placeholder tag to the markup used to build the UI, but I really don’t like this approach anymore: it’s inherently messy. As a replacement, I coded up in a couple of hours a new GtkAction subclass: GtkRecentAction; when bound to a menu item tag, it creates a new submenu hooked to a GtkRecentChooserMenu; when bound to a toolbar item tag, it creates a GtkMenuToolButton like the one Gedit uses for the Open toolbar button. Unfortunately, I still can’t see a way to hook up an inlined list of menu items. Well, I suppose I’ll have to file a bug to the HIG and have it changed to use submenus for the list of recently used files. ;-)

gnome-utils: good news everyone: I’ve asked Fabio Marzocca for his permission to include into the gnome-utils package the fine disk usage tool called Baobab that he wrote; he agreed, and work is underway to clean up the code and hook it up to the gnome-utils build, in order to have it ready for the 2.15.1 release. Also, work is underway to add Solaris support to the system log viewer, and I’m fixing up a bunch of open bugs for the dictionary, including the re-addition of the “speller tool” (the list of matching words showed if no definitions were to be found).

The API has landed

March 29, 2006 on 10:19 pm | In Hacking, GNOME, recent-files, announce | 2 Comments

The GtkRecent code is finally in gtk+ HEAD branch.

A couple of things are still missing, mostly documentation and examples (the exact list is here, if you want to check).

The code needed a year to take shape (I was beginning to mull over it a bit too much), but it is cleaner, more efficient and works better than the EggRecent code, that has been copied and pasted all around the world.

The heroes of this tale are undoubtedly: James Willcox, for writing the original code (even if he didn’t reply to my email about its state bad boy, bad boy); Federico Mena-Quintero, for listening to me while we were waiting at a terrible restourant in Stuttgart, and for saying: Go ahead, implement it, and everyone will love you!; Matthias Clasen (still no blog, Matthias?), for helping and suggesting and asking and for maintaining the Best. Toolkit. Ever.

This work is dedicated to my soon-to-be wife, Marta, and for the patience she had while I hacked till the wee hours of the morning. I love you.

And this is just the beginning… I sense bug reports coming…

Merger

March 25, 2006 on 1:36 pm | In Hacking, GNOME, recent-files | No Comments

Seems that I’m being syndacated… aggregated… whatever… on a Planet, so hi!

I’m starting merging the GBookmarkFile parser into my local copy of GLib’s HEAD branch (the tracker bug is #327662). I’ve created a testb suite and a bunch of invalid and valid bookmark files, in order to test it - and they actually helped me find a couple of stupid regressions that I had introduced.

Pending review of the patch by Matthias, the code should go in GLib first thing next week; once GBookmarkFile is in, the recent manager and the related widgets are due to go in the following days. Right now, I’m writing a patch for GTK’s HEAD branch - stay tuned for a tracker bug about it. Update 2006-03-27@00:31: bug #336121.

Whoa, just a couple weeks shy on a year since I began writing the Perl bindings for the egg-recent code - which is the reason that forced me to open that can of worms and where all the recent files rewrite really began. Never thought that it would take this long, or that it would take me this far. Well, actually I never really thought I had some chances of actually contributing something to the platform libraries at all.

Now listening: Burning down the house, Talking Heads

ChangeLog

March 22, 2006 on 1:42 am | In Hacking, GNOME, recent-files, travel, job | 1 Comment

Long time, no blog.

Here’s a brief recap of what happened in the last three weeks:

  • Matthias Clasen has began reviewing the Recent Documents code, and we are planning to get it merged beginning next week; I sent a recap to the gtk-devel mailing list, and I’ll send the draft for the Desktop Bookmark specification on the xdg mailing list at fd.o (like I did a year ago, but now I have a working implementation);
  • GNOME 2.14 is out!
  • gnome-utils 2.14.0 is out too!
  • Me and Marta went to London, last week: she went to look the premises of the London School of Economics, as she’ll be taking a MSc there starting from September; me, I met with Matthew Allum, and following the arrengements we made at FOSDEM last Februrary, I signed a contract for working with the great guys at OpenedHand.
  • And last, but not least (or I would be in trouble), I’m getting married!

Porting

January 30, 2006 on 2:59 am | In Hacking, GNOME, recent-files | No Comments

I’ve just updated the Recent Files and Bookmarks page on Gnome’s wiki: some (long due) clean ups and typo checking, but I’ve also added a porting guide, showing how to use the RecentManager and RecentChooser objects: addition, look up, display, sorting and filtering. If you are using the EggRecent code in your applications, go check it out.

As soon as Gnome 2.14 hits the mirrors, I’ll finally deprecate the EggRecent code - and won’t have to deal with the horrible EggRecentViewGtk widget anymore. Really, if there is a reason I wanted to implement the recenly used documents stuff, it surely must be to get rid of that code.

Developement Trunk

January 21, 2006 on 3:20 pm | In Hacking, GNOME, recent-files, project-ridley | No Comments

Shamed by kris’ post about the state of Project Ridley stuff, I resumed the recent-files hacking after the gnome-utils/gnome-dictionary crazy run that occupied my nights in the last three months or so.

RecentManager and RecentChooser

I already did a review of the documentation and the code last month, but I stopped before committing because I wanted to make a major overhauling of the sorting and filtering stuff - namely, the removal of the sorting and filtering functions from the RecentManager class and the implementation of custom sorting inside the RecentChooser class.

At first, I designed the RecentManager object as the only proxy for every operation on the list of recently used resources; then I designed the RecentChooser interface, in order to create a uniform API to display and access the data provided by the RecentManager. It all worked without much pain: you would instantiate a RecentChooser implementation and it would create its own manager. This, though, led to an unclean separation between model and viewer in an otherwise MVC approach. I went back to the design table, and severed each direct usage of the RecentManager API that would affect the list displayed by the RecentChooser from within the various implementations. This approach allowed the usage of the same RecentManager instance inside N RecentChooser implementations - allowing, also, to have a single, session-based, RecentManager instance shared between various processes (not coded, until we can integrate all the stuff inside GTK). The only downside in this operation was due to code replication between the RecentChooser and the RecentManager; thus, like I already did move the filtering stuff out inside the RecentFilter object, I moved the sorting stuff inside the RecentChooser API (and its implementations) and out of the RecentManager - which now just manages the list, and it’s used just to add, look up and remove resources from the list it holds.

More informations on the wiki: here and here.

LinkButton

I also did some work on the LinkButton, the widget that should supercede GnomeHref from libgnomeui. Attached to bug #314808 there’s the current implementation.

More informations on the wiki: here.

Icon

December 18, 2005 on 5:07 am | In Hacking, recent-files | No Comments

I’ve just committed my development trunk of the BookmarkFile and RecentManager objects. The former now supports custom icons for every bookmark inside the file, using the icon element; the latter, finally, supports group names for every recently used item.

I’m writing a small page for documenting how to port an application from the old EggRecent code to the new RecentManager, and also how to use the new RecentChooser widgets.

On a related note, I’ve been contacted by other developers interested about the RecentManager/RecentChooser code; expect more news soon.

Leaking/2

December 3, 2005 on 5:30 am | In Hacking, GNOME, recent-files | No Comments

After some six hours, a chinese dinner, two large glasses of earl grey tea, and a coffee, I’m finally able to declare that, according to Valgrind, both the libegg/recent-files and libegg/recentchooser code are leak-free.

Okay, take this with a grain of salt, as I’m not that good with Valgrind, and some of the logic of my widgets might lead to leaks; as far as my mallocs are concerned, my code is leak-free

[]
[]
[]

Leaking

December 2, 2005 on 6:04 pm | In Hacking, C, recent-files | No Comments

Yesterday, while I was at university waiting for a seminar about network security, I was on #gnome-it, and pbor asked me to look at a leak in EggRecentModel. I began wading through the code, but I didn’t found the leakage in time before having to leave.

In the evening, after I got back home, Marta went to her church choir practice, and I looked at recent-files/egg-recent-model.c using Valgrind (and practicing with it, since I’m not that good with this tool); using the function chain reported by valgrind, I noticed that the leak was inside the logic used by egg_recent_model_filter. This function scans the list of EggRecentItem objects built from the on-disk storage and filter out using the three pre-defined filter functions (while my RecentManager object supports only custom filtering function, and the RecentChooser implementations might filter the recently used resources list using a more complex RecentFilter object). The problem is that it used the infamous GList scan using while():

  while (list) {
    SomeData *obj = l->data;

    do something with the object

    list = list->next;
  }

This code is perfectly valid: it walks the list by exhausting it; that is: the list pointer reaches the end of the list once it reaches the end of the while. Valid code does not mean right code, though. Suppose you want to move the items of a list inside another list. By using the code above, you would have:

  GList *newlist;

  /* init the target list */
  newlist = NULL;
  while (list) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_append (newlist, obj);

    list = list->next;
  }

This code is also perfectly valid for filtering a list. Suppose, now, that you want to destroy the original list and return the filtered copy; you’ll have to free the GList using g_list_free() at the end of the while cycle:

  GList *newlist;

  /* init the target list */
  newlist = NULL;
  while (list) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_append (newlist, obj);

    list = list->next;
  }

  g_list_free (list);

  return newlist;

But we said above that the list is walked by exhausting it; hence, the list pointer is now NULL (the exit condition checked by the while cycle). Thus you are leaking the old list; to avoid this leakage, you must walk the list keeping a pointer to the list start. In order to do so, the right thing to do is to use an iterator pointer, and a for() cycle to keep the cycle compact and avoid messing up with the list = list->next assignments on every cycle break condition:

  GList *newlist, *l;

  for (l = list, newlist = NULL; l; l = l->next) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_append (newlist, obj);
  }

  g_list_free (list);
  return newlist;

This code does not leak the old list, but it leaks the data that was not moved to the new list; if the function to free the data is called some_list_free, the code above becomes:

  GList *newlist, *l;

  for (l = list, newlist = NULL; l; l = l->next) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_append (newlist, obj);
    else
      some_data_free (obj);
  }

  g_list_free (list);
  return newlist;

As a performance sidenote: if the list is long, you might consider using g_list_prepend, since it’s a constant time operation, while g_list_append walks the list; just remember to return the reversed new list:

  GList *newlist, *l;

  for (l = list, newlist = NULL; l; l = l->next) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_prepend (newlist, obj);
    else
      some_data_free (obj);
  }

  g_list_free (list);
  return g_list_reverse (newlist);

I had noticed the leak around 10:30pm and began fixing it, but I had to pick up Marta and both returned home at 11:30pm, when we went straight to bed and fell asleep almost immediately. This morning, I found bug #323002 awaiting, with a fix for the first part of the leak (thanks to Paolo). Now, libegg/recent-files HEAD has the leak fully plugged. Subsequent runs of valgrind showed no mor leaks related to the recent-files.

Now, I’m going to check my libegg/recentchooser and my libegg/bookmarkfile code for leaks.

[]
[]
[]

Long-standing

November 28, 2005 on 8:04 pm | In Hacking, GNOME, recent-files | No Comments

I’ve just committed some code that should hopefully fix bug #172587 of the old recent-files code.

The bug was hard to track down - and for it I do own a beer to Sebastien Bacher: it appears that older versions of OpenOffice creeped unescaped & into the dot-recently-used file we use to store the recent documents. The parser code inside EggRecentModel has always been a little too fragile (a mistake I tried to avoid since the beginning of my BookmarkFile parser code, by feeding it invalid XML/XBEL streams and trying to get errors instead of crashers), and thus gnome-panel had the tendency to crash until the dot-recently-used file was deleted or corrected.

I hope to close #172587 for good before marking the whole code as deprecated: it’s personal, now.

Powered by WordPress with Pool theme design by Borja Fernandez.
Entries and comments feeds. Valid XHTML and CSS. ^Top^