GNOME Bugzilla – Bug 758903
Add transfer annotations
Last modified: 2018-08-08 11:08:50 UTC
DConfClient and DConfChangeset APIs never tells if they return new reference, if they sink floating variants, etc.
Created attachment 316608 [details] [review] Add some transfer annotations
Review of attachment 316608 [details] [review]: Let's wait on this until the (transfer consume) patch is ready. ::: client/dconf-client.c @@ +402,3 @@ + * @value: a #GVariant, the value to write. If it has a floating reference it's + * consumed. + * @tag: (out) (allow-none) (transfer full): the tag from this write Hm. This should say 'nullable' these days :) ::: common/dconf-changeset.c @@ +77,3 @@ * Creates a new, empty, #DConfChangeset. * + * Returns: (transfer full): the new #DConfChangeset. Annotating _new() as transfer-full is a bit overboard, imho.
(In reply to Allison Ryan Lortie (desrt) from comment #2) > Review of attachment 316608 [details] [review] [review]: > > Let's wait on this until the (transfer consume) patch is ready. Bug #742618, I think? Given that #742618 last saw activity in February, can we go with “perfect is the enemy of good”, review and commit this patch now so it doesn’t rot, and do the (inevitable anyway) round of changes from (transfer full) to (transfer consume) once bug #742618 is fixed? > ::: common/dconf-changeset.c > @@ +77,3 @@ > * Creates a new, empty, #DConfChangeset. > * > + * Returns: (transfer full): the new #DConfChangeset. > > Annotating _new() as transfer-full is a bit overboard, imho. But necessary, because there are no heuristics about ‘*_new()’ functions automatically being (transfer full), IIRC.
(In reply to Philip Withnall from comment #3) > (In reply to Allison Ryan Lortie (desrt) from comment #2) > > Review of attachment 316608 [details] [review] [review] [review]: > > > > Let's wait on this until the (transfer consume) patch is ready. > > Bug #742618, I think? > > Given that #742618 last saw activity in February, can we go with “perfect is > the enemy of good”, review and commit this patch now so it doesn’t rot, and > do the (inevitable anyway) round of changes from (transfer full) to > (transfer consume) once bug #742618 is fixed? When Allison said that she was actually doing the patch for (transfer consume), so let's giver her a chance to finish that. That API isn't introspected yet and is going to be copied into glib itself soon, so it's not a big deal if this patch gets lost for now.
I found some mails about this hiding in my Bugzilla folder, so took a few minutes to update it. Merge request now open on GitLab: https://gitlab.gnome.org/GNOME/dconf/merge_requests/9 Let’s close this and track progress there.