From b10a4507a552d794dcf187095368dd6861a5e6c6 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Fri, 1 Nov 2024 15:35:11 +0100 Subject: [PATCH 1/2] gvariant: Use gi-docgen for the G_VARIANT_BUILDER_INIT documentation --- glib/gvariant.h | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/glib/gvariant.h b/glib/gvariant.h index a962eb536..ede16b70f 100644 --- a/glib/gvariant.h +++ b/glib/gvariant.h @@ -344,23 +344,25 @@ GQuark g_variant_parse_error_quark (void); * G_VARIANT_BUILDER_INIT: * @variant_type: a const GVariantType* * - * A stack-allocated #GVariantBuilder must be initialized if it is - * used together with g_auto() to avoid warnings or crashes if - * function returns before g_variant_builder_init() is called on the - * builder. + * A stack-allocated [struct@GLib.VariantBuilder] must be initialized + * if it is used together with + * [`g_auto()`](auto-cleanup.html#variable-declaration) to avoid + * warnings or crashes if function returns before + * [method@GLib.VariantBuilder.init] is called on the builder. * * This macro can be used as initializer instead of an * explicit zeroing a variable when declaring it and a following - * g_variant_builder_init(), but it cannot be assigned to a variable. + * [method@GLib.VariantBuilder.init], but it cannot be assigned to a + * variable. * - * The passed @variant_type should be a static GVariantType to avoid - * lifetime issues, as copying the @variant_type does not happen in - * the G_VARIANT_BUILDER_INIT() call, but rather in functions that - * make sure that #GVariantBuilder is valid. + * The passed @variant_type should be a static [type@GLib.VariantType] + * to avoid lifetime issues, as copying the @variant_type does not + * happen in the [func@GLib.VARIANT_BUILDER_INIT] call, but rather in + * functions that make sure that [struct@GLib.VariantBuilder] is valid. * - * |[ + * ```c * g_auto(GVariantBuilder) builder = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_BYTESTRING); - * ]| + * ``` * * Since: 2.50 */ From 3f71e403edeac2d0cee8aae77d2ae8e8670afcc5 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Thu, 31 Oct 2024 13:28:48 +0100 Subject: [PATCH 2/2] gvariant: Introduce G_VARIANT_BUILDER_INIT_UNSET For g_auto(GVariantBuilder) one needs to initialize it before the function returns, so it's best to do it when the variable is declared. G_VARIANT_BUILDER_INIT exists but requires specifying a GVariantType in the declaration which moves the type away from the usage of the builder which often results in less readable code. G_VARIANT_BUILDER_INIT also mentions that it's possible to explicitly zero the variable but this is hard to find and writing `g_auto(GVariantBuilder) builder = {0,};` is kind of ugly. This introduces G_VARIANT_BUILDER_INIT_UNSET which zero initializes the variable being declared. This gives us documentation and hides the explicitly zeroing detail: auto(GVariantBuilder) builder = G_VARIANT_BUILDER_INIT_UNSET (); --- glib/gvariant.h | 50 +++++++++++++++++++++++++++++++++++-------- glib/tests/gvariant.c | 22 +++++++++++++++++++ 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/glib/gvariant.h b/glib/gvariant.h index ede16b70f..4868cbacd 100644 --- a/glib/gvariant.h +++ b/glib/gvariant.h @@ -346,19 +346,19 @@ GQuark g_variant_parse_error_quark (void); * * A stack-allocated [struct@GLib.VariantBuilder] must be initialized * if it is used together with - * [`g_auto()`](auto-cleanup.html#variable-declaration) to avoid - * warnings or crashes if function returns before - * [method@GLib.VariantBuilder.init] is called on the builder. + * [`g_auto()`](auto-cleanup.html#variable-declaration). This macro can + * be used as initializer when declaring the builder, but it cannot be + * assigned to a variable. * - * This macro can be used as initializer instead of an - * explicit zeroing a variable when declaring it and a following - * [method@GLib.VariantBuilder.init], but it cannot be assigned to a - * variable. + * The effects of initializing the builder with + * `G_VARIANT_BUILDER_INIT` is the same as initializing it with + * [func@GLib.VARIANT_BUILDER_INIT_UNSET], followed by a call to + * [method@GLib.VariantBuilder.init]. * * The passed @variant_type should be a static [type@GLib.VariantType] * to avoid lifetime issues, as copying the @variant_type does not - * happen in the [func@GLib.VARIANT_BUILDER_INIT] call, but rather in - * functions that make sure that [struct@GLib.VariantBuilder] is valid. + * happen in the `G_VARIANT_BUILDER_INIT` call, but rather in functions + * that make sure that [struct@GLib.VariantBuilder] is valid. * * ```c * g_auto(GVariantBuilder) builder = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_BYTESTRING); @@ -375,6 +375,38 @@ GQuark g_variant_parse_error_quark (void); } \ } +/** + * G_VARIANT_BUILDER_INIT_UNSET: + * + * A stack-allocated [struct@GLib.VariantBuilder] must be initialized + * if it is used together with + * [`g_auto()`](auto-cleanup.html#variable-declaration). This macro can + * be used as initializer when declaring the builder, but it cannot be + * assigned to a variable. + * + * The builder can be initialized to a specific [type@GLib.VariantType] + * later with [method@GLib.VariantBuilder.init]. + * + * Use [func@GLib.VARIANT_BUILDER_INIT] to directly initialize the + * builder with a specific [type@GLib.VariantType]. + * + * ```c + * g_auto(GVariantBuilder) builder = G_VARIANT_BUILDER_INIT_UNSET (); + * + * if (condition) + * return NULL; + * + * g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{su}")); + * return g_variant_ref_sink (g_variant_builder_end (&builder)); + * ``` + * + * Since: 2.84 + */ +#define G_VARIANT_BUILDER_INIT_UNSET() \ + { \ + 0, \ + } GLIB_AVAILABLE_MACRO_IN_2_84 + GLIB_AVAILABLE_IN_ALL GVariantBuilder * g_variant_builder_new (const GVariantType *type); GLIB_AVAILABLE_IN_ALL diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 416d58f0e..12a35dad9 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -5106,6 +5106,27 @@ test_stack_builder_init_static (void) g_variant_unref (variant); } +static void +test_stack_builder_init_unset (void) +{ + GVariantBuilder builder1 = G_VARIANT_BUILDER_INIT_UNSET (); + GVariantBuilder builder2 = G_VARIANT_BUILDER_INIT_UNSET (); + GVariantBuilder builder3 = G_VARIANT_BUILDER_INIT_UNSET (); + GVariant *variant; + + g_variant_builder_clear (&builder1); + + g_variant_builder_init_static (&builder2, G_VARIANT_TYPE_BYTESTRING); + g_variant_builder_add_value (&builder2, g_variant_new_byte ('\0')); + variant = g_variant_ref_sink (g_variant_builder_end (&builder2)); + g_assert_nonnull (variant); + g_variant_unref (variant); + g_variant_builder_clear (&builder2); + + g_variant_builder_init (&builder3, G_VARIANT_TYPE_BYTESTRING); + g_variant_builder_clear (&builder3); +} + static GVariant * get_asv (void) { @@ -5973,6 +5994,7 @@ main (int argc, char **argv) g_test_add_func ("/gvariant/stack-builder-init", test_stack_builder_init); g_test_add_func ("/gvariant/stack-builder-init-static", test_stack_builder_init_static); + g_test_add_func ("/gvariant/stack-builder-init-unset", test_stack_builder_init_unset); g_test_add_func ("/gvariant/stack-dict-init", test_stack_dict_init); g_test_add_func ("/gvariant/normal-checking/tuples",