Created attachment 133782 [details] [review]
Add several DestroyNotify functions to glib-2.0.vapi.

Created attachment 133827 [details] [review]
Another option, which puts a free method in each class

A similar patch, but this one puts a free (or destroy) method in each of the classes. Requires a cast to GLib.DestroyNotify, but should prevent the warning from GCC that would result from the previous patch.

Maybe it would be better for vala to automatically create a method which maps to the free_function/unref_function?

Created attachment 133991 [details] [review]
Add free method to many classes

Same as the last one, but without the redeclaration of GLib.MappedFile.free()...

Evan, the idea in vala is that you do not need to use free. The vala compiler should make sure that no memory is leaked by freeing and destroying objects at the right time. If you have examples that that does not happen then that's a bug.

Jaap,

I gave an example--a GHashTable of GSequences. Look at the constructor:

public class HashTable<K,V> : Boxed {
	public HashTable.full (HashFunc? hash_func, EqualFunc? key_equal_func, DestroyNotify? key_destroy_func, DestroyNotify? value_destroy_func);
}

If I don't pass a DestroyNotify to them they leak, but since GSequence does not have an accessible free function, I have nothing to pass.

If you want examples that do work because the proper DestroyNotify functions are bound, I can provide those, too. A GHashTable of strings, GObjects come to mind (GLib.g_free and GLib.g_object_unref, I believe).

Look at all the places you need to pass a DestroyNotify:

nemequ@hoplite:~$ grep -c DestroyNotify /usr/share/vala/vapi/*.vapi | egrep -v '\:0$'
/usr/share/vala/vapi/dbus-glib-1.vapi:1
/usr/share/vala/vapi/gconf-2.0.vapi:2
/usr/share/vala/vapi/gdk-2.0.vapi:3
/usr/share/vala/vapi/gdk-pixbuf-2.0.vapi:2
/usr/share/vala/vapi/gio-2.0.vapi:7
/usr/share/vala/vapi/glib-2.0.vapi:15
/usr/share/vala/vapi/gnet-2.0.vapi:1
/usr/share/vala/vapi/gnome-keyring-1.vapi:2
/usr/share/vala/vapi/gnome-vfs-2.0.vapi:7
/usr/share/vala/vapi/gobject-2.0.vapi:3
/usr/share/vala/vapi/gstreamer-0.10.vapi:9
/usr/share/vala/vapi/gstreamer-base-0.10.vapi:3
/usr/share/vala/vapi/gstreamer-rtsp-0.10.vapi:1
/usr/share/vala/vapi/gtk+-2.0.vapi:22
/usr/share/vala/vapi/gtksourceview-2.0.vapi:2
/usr/share/vala/vapi/hildon-1.vapi:3
/usr/share/vala/vapi/libepc-1.0.vapi:4
/usr/share/vala/vapi/libgnome-menu.vapi:2
/usr/share/vala/vapi/libgnomeui-2.0.vapi:6
/usr/share/vala/vapi/libsoup-2.4.vapi:9
/usr/share/vala/vapi/libwnck-1.0.vapi:1
/usr/share/vala/vapi/loudmouth-1.0.vapi:5
/usr/share/vala/vapi/pangocairo.vapi:1
/usr/share/vala/vapi/pango.vapi:2
/usr/share/vala/vapi/sqlite3.vapi:2
/usr/share/vala/vapi/x11.vapi:1

That would be 97 (grep | wc -l gives 98, and one of those is the definition) places, by my count, which require an that free/unref functions are accessible in some form.

I'm certainly open to suggestions about how this could be more elegantly achieved than making exposing free/unref functions for everything, but until then I think this bug should probably remain open.

Sorry, you are right. For a hash table the straightforward solution is to use libgee instead, but I agree that because of the other functions that use DestroyNotify it might be necessary to expose the free methods

Jürg, what's your opinion on this?

I talked to Jürg about this (on IRC), the result is bug #589331. Fixing that bug would allow getting rid of all (or at least most) of the DestroyNotify arguments in vapis, and there would be no need to expose the free/unref functions.

Closing this as WONTFIX as bug 589331 has been filed.