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 684331 - Fix GValue marshalling of long and unsigned long
Fix GValue marshalling of long and unsigned long
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: GNOME 3.6
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
: 685551 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-09-18 22:14 UTC by Giovanni Campagna
Modified: 2012-10-15 13:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix GValue marshalling of long and unsigned long (2.46 KB, patch)
2012-09-18 22:14 UTC, Giovanni Campagna
none Details | Review
Fix GValue marshalling of long and unsigned long (2.03 KB, patch)
2012-09-19 04:58 UTC, Martin Pitt
rejected Details | Review
Fix GValue marshalling of long and unsigned long (2.08 KB, patch)
2012-09-19 06:00 UTC, Martin Pitt
rejected Details | Review
Fix (2.46 KB, patch)
2012-09-20 04:45 UTC, Martin Pitt
committed Details | Review

Description Giovanni Campagna 2012-09-18 22:14:57 UTC
long can be equivalent to int64 or int32, depending on the architecture,
and GI conflates this distinction in the typelib, but GType does not, and
warns if the wrong accessor is used.
Comment 1 Giovanni Campagna 2012-09-18 22:14:59 UTC
Created attachment 224677 [details] [review]
Fix GValue marshalling of long and unsigned long
Comment 2 Martin Pitt 2012-09-19 04:51:48 UTC
Thanks! There is a typo in the first hunk (you meant to say _long, not _int), otherwise this looks good. I'll commit this after the freeze.
Comment 3 Martin Pitt 2012-09-19 04:54:40 UTC
Also, I think the second and fourth hunk is wrong. get_long() for an int64 type would clip the upper half on architectures where long is 32 bits; I don't know of such architectures, but if they exist the current code looks correct.
Comment 4 Martin Pitt 2012-09-19 04:58:17 UTC
Created attachment 224696 [details] [review]
Fix GValue marshalling of long and unsigned long

Attaching the corrected patch. Thanks!
Comment 5 Paolo Borelli 2012-09-19 05:39:52 UTC
Review of attachment 224696 [details] [review]:

::: gi/pygi-argument.c
@@ +2010,3 @@
             break;
         case GI_TYPE_TAG_INT64:
             arg.v_int64 = g_value_get_int64 (value);

why did you remove "break"?
Comment 6 Martin Pitt 2012-09-19 06:00:57 UTC
Created attachment 224697 [details] [review]
Fix GValue marshalling of long and unsigned long

Because I forgot to do format-patch after "commit --amend" again, sorry. Thanks for spotting!
Comment 7 Giovanni Campagna 2012-09-19 11:12:04 UTC
(In reply to comment #3)
> Also, I think the second and fourth hunk is wrong. get_long() for an int64 type
> would clip the upper half on architectures where long is 32 bits; I don't know
> of such architectures, but if they exist the current code looks correct.

long is the size of a pointer (usually), so it's 32 bits in x86. But in that case GI would mark it (u)int32, so that branch would never be hit.
On the other hand, current code is wrong in the interesting case (x86_64), were long is marked (u)int64 but using the int64 accessors causes a critical.
Comment 8 Martin Pitt 2012-09-20 04:45:36 UTC
Created attachment 224803 [details] [review]
Fix

Ah, thanks for the additional explanation. So, attaching your first patch again with only the _int -> _long typo fixed.

Curiously, I've never seen this warning. In which kind of code did you encounter this, some property declared as long type? I thought we had a fairly good coverage of data types in the tests; I'd like to add a test case for this, but if you could point me to something that reproduces this it would help. Thanks!
Comment 9 Giovanni Campagna 2012-09-20 07:02:30 UTC
Found in signal emission of RbExtDB::request in Rhythmbox, which has a G_TYPE_ULONG parameter.
Comment 10 Martin Pitt 2012-09-24 07:53:45 UTC
Comment on attachment 224803 [details] [review]
Fix

Committed, fix will be in 3.4.1. Thanks!
Comment 11 Jonathan Matthew 2012-10-15 09:25:01 UTC
*** Bug 685551 has been marked as a duplicate of this bug. ***
Comment 12 Paul Menzel 2012-10-15 13:02:39 UTC
The Debian BTS tracks this issue in bug #660113 [1].

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=660113