After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 586319 - get_value for GtkTreeModel is broken
get_value for GtkTreeModel is broken
Status: RESOLVED FIXED
Product: seed
Classification: Bindings
Component: libseed
git master
Other Linux
: Normal normal
: ---
Assigned To: seed-maint
Depends on:
Blocks:
 
 
Reported: 2009-06-18 21:46 UTC by Xan Lopez
Modified: 2011-04-24 09:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-Fix-iteration-over-out-parameters-after-invoking-a-f.patch (1.89 KB, patch)
2009-06-22 12:27 UTC, Xan Lopez
committed Details | Review
test case (675 bytes, text/plain)
2009-07-18 23:36 UTC, Tim Horton
  Details

Description Xan Lopez 2009-06-18 21:46:14 UTC
model.get_value(iter, column, value) does not seem to put anything useful in value, basically.
Comment 1 Tim Horton 2009-06-18 23:14:07 UTC
This is /partially/ due to a lack of 'out' argument annotation for that function in gir-repository. I just added a patch to gir-repository in the /patches directory of the Seed git repo that fixes that part.

This gets us as far as (where m is a Gtk.ListStore, in this case, and iter is pointing to a valid element):

var val = {};
m.get_value(iter, 0, val);
print(val.value);

Which is a seed_struct, as it should be, but struct functions (val.value.get_int()) don't work yet as far as I recall, so you can't get the value out of that. Slightly better, but not quite there.
Comment 2 Xan Lopez 2009-06-20 22:55:28 UTC
So do you have any notion/intuition about what else is broken? This is blocking the development of some stuff I need to fix other bugs (don't you love that), so I'm willing to spend some time on it if you guys are busy.
Comment 3 Xan Lopez 2009-06-22 12:27:15 UTC
Created attachment 137169 [details] [review]
0001-Fix-iteration-over-out-parameters-after-invoking-a-f.patch

The code that collected the out parameters of function calls was totally broken AFAICT. This seems to fix it, and I've verified that with this fix we actually set a properly initialized GValue to the value variable in 'model.get_value(iter, 0, value)'.

Trying to fetch anything from it with 'value.value.get_int()' still fails though.
Comment 4 Xan Lopez 2009-06-22 15:51:10 UTC
OK, the remaining problem is one of memory corruption. seed_gobject_method_invoked allocates an array of GArguments to hold the out values, and an element of that array is, later, directly set as the 'pointer' variable of the Seed struct that is the out GValue. As we free the out_values array before exiting seed_gobject_method_invoked, that memory is invalid, and value.get_int() will fail (because the type is garbage).

As a proof, a hack like this:

@@ -468,7 +477,7 @@ seed_gobject_method_invoked (JSContextRef ctx,
       else if (dir == GI_DIRECTION_OUT)
        {
          GArgument *out_value = &out_values[n_out_args];
-         out_args[n_out_args++].v_pointer = out_value;
+         out_args[n_out_args++].v_pointer = g_new0(GArgument, 1);

"fixes" the problem. Not sure what's the best fix for this, but I suspect we need more fine grained support for GValues than what we have in seed-structs.c.
Comment 5 Bryan Forbes 2009-07-17 16:53:03 UTC
Actually, I just changed my GIR repository's Gtk entry's gtk_tree_model_get_value to this:

      <method name="get_value" c:identifier="gtk_tree_model_get_value">
        <return-value transfer-ownership="none">
          <type name="none" c:type="void"/>
        </return-value>
        <parameters>
          <parameter name="iter" transfer-ownership="none">
            <type name="TreeIter" c:type="GtkTreeIter*"/>
          </parameter>
          <parameter name="column" transfer-ownership="none">
            <type name="int" c:type="gint"/>
          </parameter>
          <parameter name="value" direction="out" transfer-ownership="none">
            <type name="GObject.Value" c:type="GValue*"/>
          </parameter>
        </parameters>
      </method>

(effectively, I applied the patch without re-compiling GIR).  After that, I can do this:

var store = new Gtk.ListStore(), iter = new Gtk.TreeIter(), value = GObject.Value();
store.set_column_types(1, [GObject.TYPE_STRING]);

store.append(iter);
store.set_value(iter, 0, "foo");
store.append(iter);
store.set_value(iter, 0, "bar");
store.append(iter);
store.set_value(iter, 0, "baz");

store.get_iter_first(iter);
store.get_value(iter, 0, value);
print(value.get_string()); // prints foo

store.iter_next(iter);
store.get_value(iter, 0, value);
print(value.get_string()); // prints bar

store.iter_next(iter);
store.get_value(iter, 0, value);
print(value.get_string()); // prints baz

Does this mean that once GIR is fixed, this bug will be fixed?
Comment 6 Tim Horton 2009-07-18 23:35:48 UTC
Bryan: I can't reproduce that with any combination of Xan's patch and the GIR patch (which generates exactly the same content as you specify above). My test script (which is just yours with init stuff and "new GObject.Value()") fails with lots of:

(seed:24095): GLib-GObject-CRITICAL **: g_value_get_string: assertion `G_VALUE_HOLDS_STRING (value)' failed

Xan: Though I'm not familiar at all with this part of the codebase (and am hoping Robb will fix the remainder of this when he returns), your patch makes sense, so I committed it.
Comment 7 Tim Horton 2009-07-18 23:36:44 UTC
Created attachment 138713 [details]
test case
Comment 8 Bryan Forbes 2009-07-20 19:18:52 UTC
I didn't do the patch that Xan posted.  Only the GIR change.  I'm using seed and GIR from the orange-owners PPA.
Comment 9 Alan Knowles 2010-01-25 10:38:48 UTC
using inout seems to fix the warnings. - this needs to go upstream I guess.

  <parameter name="value" direction="inout" transfer-ownership="none">
            <type name="GObject.Value" c:type="GValue*"/>
          </parameter>
Comment 10 Alan Knowles 2011-04-24 09:19:41 UTC
This has been fixed now - it uses caller_allocates and is an 'out' property