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 770077 - Custom GParam flags usage is unclear and possible bug in documentation of maximum user shift
Custom GParam flags usage is unclear and possible bug in documentation of max...
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gobject
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2016-08-18 08:51 UTC by Heinrich Fink
Modified: 2018-05-24 19:03 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Heinrich Fink 2016-08-18 08:51:04 UTC
In gobject/gparam.h, the documentation of G_PARAM_USER_SHIFT defines the maximum user shift count to be 10:

/**
 * G_PARAM_USER_SHIFT:
 * 
 * Minimum shift count to be used for user defined flags, to be stored in
 * #GParamSpec.flags. The maximum allowed is 10.
 */

Since the shift count is set to 8, this effectively means we can't define more than three user-defined flags, which doesn't seem very practical.

Also, it would be good to know, why G_PARAM_EXPLICIT_NOTIFY and G_PARAM_DEPRECATED start at shift-count 30, and why those flags are not simply continued directly after G_PARAM_STATIC_BLURB.

This seems all a bit unclear and confusing to me. Does someone have an explanation for this, or is this simply a bug in the documentation?
Comment 1 Sebastian Dröge (slomo) 2016-08-18 08:53:40 UTC
Also this causes potential problems with other code, e.g. GStreamer defines some flags above G_PARAM_USER_SHIFT (G_PARAM_USER_SHIFT + 1, 2, 3, 4) and has a GST_PARAM_USER_SHIFT on top of that (+8 = 16).
Comment 2 Emmanuele Bassi (:ebassi) 2016-08-18 09:15:37 UTC
While we can, and should, definitely improve the documentation, putting an explanation in would require transcribing the history of the project, which is somewhat out of scope for an API reference.

The long and short of it is:

  Nobody should be using user flags in GParamSpec in newly written code

in practice, though, it's a somewhat useful feature that has been used in the past and needs to be supported for the whole duration of the GLib 2.0 ABI.

G_PARAM_DEPRECATED was added at the end of the enumeration to avoid breaking code that used G_PARAM_USER_SHIFT, like GIMP. We cannot compatibly add further flags after G_PARAM_STATIC_BLURB — i.e. we cannot change the value of G_PARAM_USER_SHIFT — so we need to add them from the end of the enumeration's space "inwards".

> Since the shift count is set to 8, this effectively means we can't define more
> than three user-defined flags, which doesn't seem very practical.

Ideally, you'd be able to define your user GParamSpec flags as:

  #define YOUR_PSPEC_FLAG_FOO (1 << (G_PARAM_USER_SHIFT + 1)
  #define YOUR_PSPEC_FLAG_BAR (1 << (G_PARAM_USER_SHIFT + 2)

This means that you have (G_PARAM_USER_SHIFT - last_defined_enumeration_value) slots available. In practice, we want to avoid people doing so, because makes future additions to the enumeration basically impossible. We won't be breaking GLib ABI any time soon (or possibly ever) which means that we need room to expand. No user of G_PARAM_SPEC_USER_SHIFT has ever added more than a couple of values, so we decided to settle for three slots. That does not mean you cannot add more values, because we cannot have any sensible check in place to warn you about it; but you're gently nudged in the direction of not doing so.
Comment 3 Emmanuele Bassi (:ebassi) 2016-08-18 09:17:57 UTC
(In reply to Sebastian Dröge (slomo) from comment #1)
> Also this causes potential problems with other code, e.g. GStreamer defines
> some flags above G_PARAM_USER_SHIFT (G_PARAM_USER_SHIFT + 1, 2, 3, 4) and
> has a GST_PARAM_USER_SHIFT on top of that (+8 = 16).

I know that GStreamer has the ability to find and exploit any esoteric feature of GObject, but using and exposing additional GParamFlags should have been a red flag from day one. :-)

GParamSpec has support for GData so it would probably be better to use values stored that way instead of tweaking the enumeration.
Comment 4 Heinrich Fink 2016-08-18 09:42:00 UTC
Thanks for the clarification.

> GParamSpec has support for GData so it would probably be better to use
> values stored that way instead of tweaking the enumeration.

I guess you meant g_param_spec_set_qdata and related, right? We already use that in our application to indicate the element type of GArray properties to avoid using GValueArray. 

So we'll just avoid using custom flags then and set our custom flags as qdata as well.
Comment 5 GNOME Infrastructure Team 2018-05-24 19:03:21 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/1194.