The many issues of GLib.Datalist
I wanted to use GLib.Datalist in one of my programs, but it turned out to be rather broken when used from Vala.
Direct issues I've encountered
Let's go with this simple program:
void my_inserter(Datalist<string> dlist) {
dlist.set_data("good", "day");
}
int main() {
var dlist = new Datalist<string>();
dlist.set_data("hello", "world");
my_inserter(dlist);
print("Listing results:\n");
dlist.foreach((quark, data) => {
print("%s %s\n", quark.to_string(), data);
});
return 0;
}
How many problems are there? At very least three, one of them very serious!
Memory corruption due to access from function
I'm not quite sure why this happens, but appending to datalist in my_inserter
leads to memory corruption logged by valgdrind and quite possibly segfault. Using ref
argument seemingly solves the problem, but this remains a very viable method to shoot yourself in your foot by merely passing a structure to a function and getting no compiler warnings.
Datalist itself is never freed
It's not very clear from documentation, but you need to release datalist contents by calling g_datalist_clear
as its destructor. Trivial to fix - just annotate Datalist with [CCode (destroy_function = "g_datalist_clear")]
.
Even with fix above strings are leaked anyways
This is because set_data
assumes that no freeing is needed (i.e. performs no ownership transfer) while its argument annotation points to the contrary. I've tried to annotate method in question as
[CCode (cname = "g_datalist_set_data_full", simple_generics = true)]
public void set_data (string key, owned G data);
...which should just work as intended from what I read, but this led to Vala compiler bailing out from somewhere deep inside and I'm not up to the task of debugging that.
As a side note, it might be useful to keep unowning setter, although it probably should not be the default one (set_data_unowned
?). Finally, all in this section also applies to quark version of this function, id_set_data
.
Even more issues to ponder
id_replace_data
should probably just be dropped from binding
It has complex ownership management semantics that Vala simply does not support. Worse yet, they are conditional - that is, you need to check its return value to know which value you own and which you do not.
DuplicateFunc
should use nullable types
Accessors and This includes both argument and return value of DuplicateFunc
delegate and return values of methods id_dup_data
, id_get_data
, id_remove_no_notify
, get_data
, remove_no_notify
.
Finally - should Datalist itself be a generic? Or only its member functions?
Datalist allows you to store multiple different types in it by design, so treating it as storing only a single type only sometimes makes sense (notably in its use in libsoup, where keys are expected to be strings). It might be a good idea to introduce another type (kind of like GenericSet
for HashTable
) that would have member functions take generic argument rather than class itself. Although I'm not sure what foreach
function would look like in such implementation...