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 774669 - Generic array conversion produces incorrect array with enumerations and flags
Generic array conversion produces incorrect array with enumerations and flags
Product: libgee
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: libgee-maint
: 776536 (view as bug list)
Depends on:
Reported: 2016-11-18 12:14 UTC by Colomban Wendling
Modified: 2016-12-30 07:07 UTC
See Also:
GNOME target: ---
GNOME version: ---

Fix converting enumerations and flags to arrays (4.37 KB, patch)
2016-11-18 12:14 UTC, Colomban Wendling
committed Details | Review

Description Colomban Wendling 2016-11-18 12:14:31 UTC
Created attachment 340231 [details] [review]
Fix converting enumerations and flags to arrays

The generic array conversions fail to special-case enumerations and flags type, which most of the time results in incorrect arrays.  It is the same underlying issue as

This issue breaks real apps in hard-to-understand ways, see for example

Attached patch adds special handling for those so they are (generally) correctly handled.  Below is the patch's description:


Fix converting enumerations and flags to arrays

Enumerations and flags are classed types for Vala, not integers, so they don't fall in the `typeof(G) == typeof(int)` kind of tests. This leads to using the generic code in which Vala assumes pointer-sized elements, which is often not true for enumerations and flags.

Add special case for those to use the `int` converters for enumerations and flags.

This is most generally correct, but not always: the compiler will likely chose a larger type for a specific enumeration if one of its value is larger than `int`.  It would be tempting to use the enumeration's class minimum and maximum values to determine the appropriate type, but unfortunately the API for this uses int itself, so doesn't help.
Comment 1 Rico Tzschichholz 2016-11-23 18:23:41 UTC
Overall this seems ok, although the added test-case fails (on 64bit).
Comment 2 Colomban Wendling 2016-11-23 18:33:08 UTC
(In reply to Rico Tzschichholz from comment #1)
> although the added test-case fails (on 64bit).

Does it?  It works good here, although I apparently have to run `make` before `make check` (or `make check` twice) to get the tests use the new version of the library.  But on all subsequent runs it succeeds just fine, so I guess it's a build system dependency issue.
Comment 3 Rico Tzschichholz 2016-11-23 19:59:53 UTC
OK, the testsuite isn't using the just built library but the installed one and therefore obviously fails for that reason. So the test succeeds now.
Comment 4 Maciej (Matthew) Piechotka 2016-12-10 19:19:37 UTC
Review of attachment 340231 [details] [review]:

Looks good.
Comment 5 Rico Tzschichholz 2016-12-13 18:03:15 UTC
Attachment 340231 [details] pushed as 72671f6 - Fix converting enumerations and flags to arrays
Comment 6 Michael Gratton 2016-12-30 07:07:15 UTC
*** Bug 776536 has been marked as a duplicate of this bug. ***