GNOME Bugzilla – Bug 683596
64 bit integer not properly converted from gvalue
Last modified: 2012-09-12 04:51:52 UTC
Created attachment 223792 [details] [review] argument: Fix 64bit integer convertion from gvalue The 64 bit integers are not retreive properly from GValues, as a result, an assert and 0 being returned. You'll find attached a patch that fixes this issue.
we will need a unit test for this (if needed add the C part to gobject-introspection)
Sure, I'm just sharing my findings here. In gobject-instrospection, this aspect seems covered by the tests. I also came across this test, which I'm not sure it right on 32bit machines. pygi-argument.c:58 case GI_TYPE_TAG_INT64: *gssize_out = arg_in->v_int64; return TRUE; case GI_TYPE_TAG_UINT64: *gssize_out = arg_in->v_uint64; return TRUE;
Thanks for this. The patch looks fine, but I'd really like to capture this in a test case. At least GValue int64 already works fine with the current PyGObject as normal method arguments, both in and out. I just committed test API for that to gobject-introspection: http://git.gnome.org/browse/gobject-introspection/commit/?id=9a472fb76c8d603b439562accf0593bb63d86215 and tests to pygobject that make sure that these work: http://git.gnome.org/browse/pygobject/commit/?id=822d9e07a95f706a40f64335765293542787da90 _pygi_argument_from_g_value() is only used for unmarshalling signal arguments, so I'll add a test for that.
I'm afraid I still can't reproduce this error. Can you please attach or link to a piece of code which does not work? - int64 GValue input arguments and return values for methods work: http://git.gnome.org/browse/pygobject/commit/?id=822d9e07 - int64 input arguments and return values for signals work: http://git.gnome.org/browse/pygobject/commit/?id=15046b5a - int64 GValue return values for signals work: http://git.gnome.org/browse/pygobject/commit/?id=fddb01b - int64 GValue input arguments for signals work if you construct them explicitly: http://git.gnome.org/browse/pygobject/commit/?id=4559247553b - int64 GValue signal arguments do not work if you only specify a bare constant, but there the problem is the other direction (marshalling from Python to C), and it's not 100% clear that this should be fixed. See previous patch; I captured this in bug 683775. Thanks in advance!
(In reply to comment #4) > - int64 GValue signal arguments do not work if you only specify a bare > constant, but there the problem is the other direction (marshalling from Python > to C), and it's not 100% clear that this should be fixed. See previous patch; I > captured this in bug 683775. I'm a bit unclear of what you mean by "it's not 100% clear that this should be fixed". From reading the code, it's quite clear that calling g_value_get_uint() on a gvalue that holds a GUINT64 will never work. Are you saying you have a patch that completly remove that code ? For you interest, the situation is gst-editing-services that emit from C the snapping signal. This signal has a GstClockTime (guint64) as parameter. This code is being triggered when the signal is being relayed to Python. Unfortuatly, this is the oppsite of what you just described, which might explain why you don't reproduce it. Taking that code out and running a test like the following clearly show the code is wrong. >GValue v = {0} >g_value_init (&v, G_TYPE_UINT64); >g_value_set_uint64 (&v, 1); >g_assert (g_value_get_uint (&v) == 1); returns: >(process:24520): GLib-GObject-CRITICAL **: g_value_get_uint: assertion `G_VALUE_HOLDS_UINT (value)' >failed >** >ERROR:test.c:12:main: assertion failed: (g_value_get_uint (&v) == 1) >Aborted This is exactly what that code is doing.
(In reply to comment #5) > (In reply to comment #4) > > - int64 GValue signal arguments do not work if you only specify a bare > > constant, but there the problem is the other direction (marshalling from Python > > to C), and it's not 100% clear that this should be fixed. See previous patch; I > > captured this in bug 683775. > > I'm a bit unclear of what you mean by "it's not 100% clear that this should be > fixed". Oh, *this* bug (683596) should certainly be fixed. I meant, it's not clear whether bug 683775 is really a bug or not. > From reading the code, it's quite clear that calling g_value_get_uint() > on a gvalue that holds a GUINT64 will never work. Are you saying you have a > patch that completly remove that code ? No, I just have trouble finding a reproducer which actually triggers that faulty code. It's obviously wrong. > For you interest, the situation is gst-editing-services that emit from C the > snapping signal. This signal has a GstClockTime (guint64) as parameter. This > code is being triggered when the signal is being relayed to Python. > Unfortuatly, this is the oppsite of what you just described, which might > explain why you don't reproduce it. Right, I see. Thanks!
Created attachment 224047 [details] [review] regress: Add utility to test signals with int64 and uint64 params Changes required to gobject-introspection Regress module to test int64 signal parameters. I've based my work on the skeleton that Martin wrote today.
Created attachment 224049 [details] [review] test: Add test for signal with int64 and uint64 parameters This test covers the bug being fixed in this patch.
Comment on attachment 224049 [details] [review] test: Add test for signal with int64 and uint64 parameters This is the wrong patch, it's the actual bug fix again, not the test. But I had it mostly written yesterday anyway, so thanks!
Comment on attachment 224047 [details] [review] regress: Add utility to test signals with int64 and uint64 params Committed the g-i test with some cleanup (fixing the expected output and dropping reference to the nonexisting regress_test_obj_emit_sig_with_int64_no_return). Thanks!
Comment on attachment 223792 [details] [review] argument: Fix 64bit integer convertion from gvalue Committed the fix with test cases. http://git.gnome.org/browse/pygobject/commit/?id=4e4c87e3868948743e0446abe2ba0cf5626374c4 Thanks!