Documentation Improvement for GValue and Boxed types

Platform
1

Keyword: GValue, Boxed Type

Background

I wanted to store arbitrary values and types in a GHashTable. For that I used GValue and later GValue with a Boxed Type. I had a very hard time figuring out how everything works and I’d like to improve it. Therefore I opened this discussion.

Why don’t I file merge request?

There’s a high chance, I misunderstand things or don’t see the overall picture, so MRs might be a waste of time for both sides. Instead I’d like to try and get together the view and questions of a novice (me) and the answers and knowledge of a pro to come up with an improved content.
After the ok on a specific subject below, I could create an MR. (Althouth, someone with better writing skills is very welcome to make changes instead :slight_smile: )

Points of Discussion

Minor: There’s no “GBoxed” (devhelp) site (/entry)

Problem:
Searching in Devhelp for “GValue” brings up information on it. However, there’s no page page when searching for “GBoxed”. Instead one has to search for “Boxed Types”.

Suggestion:
Create an entry for GBoxed, that at least links to the “Boxed Types” page - and possibly explains, why GBoxed does not exist.

GValue

Minor: Two entries “GValue”

Problem:
“GValue” has basically two entries:

  • [1], containing functions like g_value_init, g_value_copy, etc
  • [2], containing functions like g_value_set_string, g_value_get_string, …

Suggestion:

  • Difficult: If possible, merge the information into one site
    Or
  • Simple: Add See also right below Description of each site and link the other one.

[1] Generic values: GObject Reference Manual

[2] Parameters and Values: GObject Reference Manual

It’s not clear, that g_value_unset() is needed

Problem:

  • [2] says, that g_value_init has to be used, but there is no word of g_value_unset.
  • Naturally “init” suggests the existence of “uninit”, while “set” suggests “unset”. This makes it even more difficult /to spot/, that you need unset

Suggestion:

  • Add explicit information under Description, that unset needs to be called.
  • For future reference, the opposite of init might better be uninit. Maybe it’s even worth thinking of creating an alias uninit for unset

[2] Parameters and Values: GObject Reference Manual

Memory management of GValue:

Problem:
GValue will work completely differently, depending on the type. Here some (assumed) examples.

  • G_TYPE_CHAR: owned value; stack, no unset necessary
  • G_TYPE_INT, …: owned value; stack; no unset necessary
  • G_TYPE_BOOLEAN: owned value; stack; no unset necessary
  • G_TYPE_STRING:
    • static: not owned value; heap; no unset necessary
    • not static: owned value; heap; unset call becomes necessary
  • G_TYPE_OBJECT: owned value; heap; no unset necessary (or did I forget to g_object_ref in my test?)
  • G_TYPE_BOXED: ownage depends on implementation of specific type; unset necessary
  • G_TYPE_POINTER: not owned; heap; not owned

Suggestion:
Each of these types has a g_value_set_* function. Some also have g_value_take_*, g_value_set_static_*, g_value_set_*_take_ownership.
In each function description, make it clear, if an unset() call is needed

Major: Unify language: set/take is arbitrary

( This would be an interface change, so chances are low, that anyone will support it. But I can’t let it be unmentioned. )

Problem:
Functions names and their meaning feels arbitrary. Here are examples:

  • _set_string copies the given string.
  • _set_pointer takes the given pointer.
  • _set_boxed copies the given pointer.
  • _set_int (& co) takes the given pointer.
  • _set_param … copies (?) the given param… I guess… because there is a take function, but the function description doesn’t say anything about it

Suggestion
Unify function names and their meaning.

Add Example on how to use boxed types with GValue

Problem
Especially with the very, very scrace information on boxed types, it was very difficult to figure out how to use a boxed type - and why to use a boxed type.

Suggestion:

  • Add information: "GValue does not provide a destroy function for arbitrary types (G_TYPE_POINTER). If you want to use a GValue with an arbitrary type, wrap it as a G_TYPE_BOXED"

  • Add an example of how to use GValue with a boxed type close to the text above. Or maybe better on the page for Boxed Types and create a link from the text above.

GType BOXED_STRING = g_boxed_type_register_static (
                   "boxedString", 
                   boxed_string_copy, 
                   boxed_string_free);

  GValue * val = g_new0(GValue, 1);
  g_value_init (val, BOXED_STRING);
  g_value_take_boxed (val, g_strdup ("hello"));
  g_value_unset (val);
  g_value_free (val);
1 Like
2

Sorry for the slow reply, but thank you for spending the time to diagnose the issues you’ve had with the documentation and write them all up. It’s not often we get enthusiastic feedback like this. :slightly_smiling_face:

Searching for GBoxed does bring up the GBoxedCopyFunc and GBoxedFreeFunc documentation for me, which are on the right page.

I think the fix here is to add full-text search to Devhelp (currently it only searches page titles and possibly keywords). Please file an issue against Devhelp about that, if one doesn’t exist already.

A shorter-term fix might be to add an additional indexterm element to the ‘Boxed Types’ docbook page, although that’s a tentative fix because I’m not sure how to do that in gtk-doc (which is what GLib uses to generate its documentation) and I’m not sure whether Devhelp searches the index terms.

Merge requests welcome. :slightly_smiling_face:

The first page is the one which describes GValue, the second is the tediously long list of all the different types it supports. I suspect they are separate because a combined page would be too long to usefully be able to find anything in.

I suspect the fix for this in the long term is to use something with a less linear structure than gtk-doc for documenting GLib.

In the meantime, your suggestion of adding ‘See also’ links would help; please feel free to open a merge request. :slightly_smiling_face:

Yes, g_value_unset() should definitely be mentioned in the GValue ‘description’ section. Merge request welcome for that.

Renaming g_value_unset(), or even providing a wrapper for it with a new name, is not going to happen, as it would be incredibly disruptive to existing code (by triggering a lot of deprecation warnings which maintainers have to fix, without the new function providing any benefit to them).

unset is a more consistently used term within the GNOME/GLib ecosystem than uninit, so once people have learned that it means to clear the internal data from a structure without freeing the structure itself, that knowledge can safely be applied consistently to other GLib-style APIs.

It’s safest to assume that an unset() call is always needed, regardless of the type. One of the purposes of GValue is to be able to dynamically marshal different types through the same code, so the code can’t necessarily assume that a GValue will have a certain type and hence will/will not need unset() called on it.

If the type information was known at compile time, there would be little point in using GValue — the code might as well use the underlying wrapped C type.

I think you might be misunderstanding here. The function naming is consistent. set functions set the GValue to a copy of their argument. In the case of non-pointer types (integers, booleans), this is a copy-by-value. In the case of pointer types, this is a copy by whatever copy function is relevant to the type (g_strdup() for strings, g_object_ref() for GObjects, etc.). set_pointer treats the pointer as an opaque value; if you need more specialised handling for a particular pointer type, it must be boxed using GBoxed, which exposes the ref/unref functions for that type to the runtime type system.

take functions set the GValue to their argument without copying, on the assumption that ‘ownership’ of the argument has been passed to the GValue.

Merge request welcome. :slightly_smiling_face:

3 Likes
3

For the record: I created two merge requests:
[1] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2178
[2] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2179

@ GValue renaming

I still feel unsatisfied here, but in regard of this thread, I feel it’s right to let it rest and check if it hits again).

3 Likes
4

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.