GNOME Bugzilla – Bug 755766
gvalue: The g_auto cleanup function assert if value is G_VALUE_INIT
Last modified: 2015-10-05 13:32:03 UTC
This snippet cause an assertion: { g_auto(GValue) value = G_VALUE_INIT; } I think it should be possible to run the cleanup function on a GValue that has only been initialized (just like NULL for autoptr).
I agree. We have a couple of possibilities here: (1) add g_value_clear() that works on unset GValue (2) add a G_DEFINE_AUTO_ variant that takes a check function. eg: G_DEFINE_AUTO_CLEAR_WITH_CHECK(GValue, g_value_unset, G_IS_VALUE) but it would be nice if this "function" were a pure macro. G_IS_VALUE currently turns into a call to the (function) g_type_check_value() which is a bit silly when all we _really_ want to do for this particular case is check if the ->g_type != 0. We could maybe think about doing another round of GType macro fast-path-additions, but to avoid getting into that, let's just add a g_value_clear() as a normal function and use that instead.
forgot to mention my reason for "would be nice if it were a pure macro" -- on fastpath exits before the value has been set (which is probably exactly the case that this bug is about) we could completely avoid any work since gcc would statically know that the initial value has not been modified.
Created attachment 312331 [details] [review] glib: Add 2.48 availibity macros
Created attachment 312332 [details] [review] gvalue: Add g_value_clear method This method is similar to g_value_unset() but will accept an uninitialized (zero-filled) GValue structure.
Created attachment 312333 [details] [review] gvalue: Use g_value_clear as clear function This change allow leaving a scope before g_value_init() has been called. This would happen if you do: { g_auto(GValue) value = G_VALUE_INIT; } Or have a return statement (due to failure) before the part of your code where you set this GValue.
Review of attachment 312331 [details] [review]: Thanks
Review of attachment 312332 [details] [review]: Looks good -- the only thing I might also do is add a note to the _unset() documentation that you cannot do this on a GType which is already empty. That can be a separate commit.
Review of attachment 312333 [details] [review]: good.
(In reply to Ryan Lortie (desrt) from comment #7) > Review of attachment 312332 [details] [review] [review]: > > Looks good -- the only thing I might also do is add a note to the _unset() > documentation that you cannot do this on a GType which is already empty. > That can be a separate commit. Good idea.
Created attachment 312357 [details] [review] gvalue: Improve _unset() documentation g_value_unset() only works with initialized value and will assert if the GValue is zero-filled (or initialized with G_VALUE_INIT). Document this behaviour and refer to g_value_clear() for a method that work on both initialized and zero-filled GValue.
Attachment 312331 [details] pushed as b36b494 - glib: Add 2.48 availibity macros Attachment 312332 [details] pushed as 1233962 - gvalue: Add g_value_clear method Attachment 312333 [details] pushed as 3bb2e8d - gvalue: Use g_value_clear as clear function Attachment 312357 [details] pushed as 3c0d38d - gvalue: Improve _unset() documentation
Why don't we just let g_value_unset() accept 0-filled GValues? We already have a slightly-confusing distinction between g_value_unset() and g_value_reset(), we really shouldn't have a third such function.
or deprecate g_value_unset() since g_value_clear() is a more common naming.
(In reply to Dan Winship from comment #12) > Why don't we just let g_value_unset() accept 0-filled GValues? We already > have a slightly-confusing distinction between g_value_unset() and > g_value_reset(), we really shouldn't have a third such function. I agree. Lets make unset for for this.
fixed in master
*** Bug 755636 has been marked as a duplicate of this bug. ***