search

conformal / gotk3

Go bindings for GTK3
ISC License
470 stars 81 forks source link

Added ListStore.InsertBefore() and InsertAfter() #37

Closed weberc2 closed 10 years ago

jrick commented 10 years ago

This is incorrect. If you look at the documentation (here: https://developer.gnome.org/gtk3/stable/GtkListStore.html#gtk-list-store-insert-before), the 'iter' variable needs to be an unset iterator. It's probably a better idea to create an unset iterator inside the function, pass a pointer to cgo to set it, and then wrap and ref count it as needed, and return it to the go caller. The docs don't specify, but I'm willing to bet that if you passed in a valid iterator to this func, it would leak the old iterator.

weberc2 commented 10 years ago

If this is incorrect, aren't Append() and Prepend() similarly incorrect as well? I'll try and make the requisite changes tonight.

jrick commented 10 years ago

They would be, yes. Nice catch.

weberc2 commented 10 years ago

@jrick How does this look?

jrick commented 10 years ago

Tests fail: http://sprunge.us/MCCb

That's with commits 8bdcd12 and 3c87e57 cherrypicked onto our master.

jrick commented 10 years ago

Simple fix, check for a nil receiver in (*TreeIter).Native.

jrick commented 10 years ago

Seeing some signals occurring during cgo execution (panics the go runtime) with another gotk3 app after the api updates. Checking...

jrick commented 10 years ago

I see the issue. Runtime finalizers are being set when they should not be. According to https://developer.gnome.org/gtk3/stable/GtkTreeModel.html#gtk-tree-iter-free, free should only be called for a copied iterator.

jrick commented 10 years ago

Fixed and committed as 556358ac40fa33c64f558b206a701e07e7cf0476.