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 686662 - introduce explicitly-sized enum types, deprecate G_TYPE_ENUM
introduce explicitly-sized enum types, deprecate G_TYPE_ENUM
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gobject
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 663054 686666 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-10-22 21:01 UTC by Colin Walters
Modified: 2018-05-24 14:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests/signals: Disable large enumeration value test that is failing on PPC64 (1.38 KB, patch)
2012-10-25 19:41 UTC, Colin Walters
committed Details | Review
Python script to query GIR files (1.44 KB, text/plain)
2012-10-25 19:44 UTC, Colin Walters
  Details

Description Colin Walters 2012-10-22 21:01:16 UTC
See https://bugzilla.redhat.com/864196 for downstream discussion.

The executive summary is that in for enums in C, the compiler will choose a storage type (unsigned int, long long) based on what the enum contains.  For GCC, if the enum's values fit in the range of 0-G_MAXUINT, unsigned int will be chosen.  This fits "most" enums.  If you have negative numbers, but the range fits into G_MAXINT, you get an int, unsurprisingly.

A case that's not handled at all by G_TYPE_ENUM is if you have > G_MAXINT values.  GCC will choose e.g. guint64, but the public enum API is only 32 bit sized.  

A corner case here is that enums that fit in G_MAXUINT but not G_MAXINT will fail on some architectures like PPC64, which is the issue linked above.

Now, what we need to do is add types so that library users can explicitly specify the size of the enum.  This is going to be tedious and painful; but I suspect most library users will just need to do s/G_TYPE_ENUM/G_TYPE_ENUM_UINT/.

Still do be done for this is an analysis of which modules in GNOME need patching.
Comment 1 Allison Karlitskaya (desrt) 2012-10-22 22:21:22 UTC
*** Bug 686666 has been marked as a duplicate of this bug. ***
Comment 2 Allison Karlitskaya (desrt) 2012-10-22 22:22:56 UTC
some notes:

GEnumValue contains 'gint value';

GSettings bindings map to/from properties of G_TYPE_ENUM.
Comment 3 Colin Walters 2012-10-25 19:41:45 UTC
Created attachment 227300 [details] [review]
tests/signals: Disable large enumeration value test that is failing on PPC64

Basically due to a combination of va_args semantics around
signed/unsigned ints, this test case fails on ppc64.  At the moment,
we have as yet to find any real-world consumer with such a large
enumeration value.

Unfortunately, the possible fixes for this are extremely invasive;
we would have to define a new enum API.

Given both of these facts, we believe it makes the most sense at the
current time to simply not test this. If we at a later time determine
there is such a real-world consumer, we can look at doing the
necessary fixes.
Comment 4 Colin Walters 2012-10-25 19:44:41 UTC
Created attachment 227301 [details]
Python script to query GIR files

Just for reference, we used this script to search through the GIR files from current gnome-ostree 3.8, and didn't find any enums larger than G_MAXINT.
Comment 5 Dan Winship 2012-10-31 17:23:50 UTC
(In reply to comment #0)
> Now, what we need to do is add types so that library users can explicitly
> specify the size of the enum.  This is going to be tedious and painful; but I
> suspect most library users will just need to do
> s/G_TYPE_ENUM/G_TYPE_ENUM_UINT/.

or just declare G_TYPE_ENUM to be uint-sized and add G_TYPE_ENUM64
Comment 6 Emmanuele Bassi (:ebassi) 2012-10-31 18:06:42 UTC
(In reply to comment #5)
> (In reply to comment #0)
> > Now, what we need to do is add types so that library users can explicitly
> > specify the size of the enum.  This is going to be tedious and painful; but I
> > suspect most library users will just need to do
> > s/G_TYPE_ENUM/G_TYPE_ENUM_UINT/.
> 
> or just declare G_TYPE_ENUM to be uint-sized and add G_TYPE_ENUM64

I'd +1 Dan's proposal: it's closest to the C spec, and to the GLib type system (int/int64, uint/uint64).
Comment 7 Colin Walters 2012-10-31 19:47:14 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #0)
> > > Now, what we need to do is add types so that library users can explicitly
> > > specify the size of the enum.  This is going to be tedious and painful; but I
> > > suspect most library users will just need to do
> > > s/G_TYPE_ENUM/G_TYPE_ENUM_UINT/.
> > 
> > or just declare G_TYPE_ENUM to be uint-sized and add G_TYPE_ENUM64
> 
> I'd +1 Dan's proposal: it's closest to the C spec, and to the GLib type system
> (int/int64, uint/uint64).

Ok, but that plan is deferred - for now, can someone just OK the patch to disable the test?
Comment 8 Ray Strode [halfline] 2012-10-31 21:13:34 UTC
Comment on attachment 227300 [details] [review]
tests/signals: Disable large enumeration value test that is failing on PPC64

You wrote the test originally, there's no way the patch could introduce regressions, I don't think you should need to wait for a rubberstamp to get things working better for now on ppc64
Comment 9 Colin Walters 2012-10-31 21:46:22 UTC
Comment on attachment 227300 [details] [review]
tests/signals: Disable large enumeration value test that is failing on PPC64

Attachment 227300 [details] pushed as 4447d5c - tests/signals: Disable large enumeration value test that is failing on PPC64
Comment 10 Colin Walters 2013-06-13 11:37:46 UTC
*** Bug 663054 has been marked as a duplicate of this bug. ***
Comment 11 GNOME Infrastructure Team 2018-05-24 14:43:48 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/617.