GNOME Bugzilla – Bug 741167
gdbus-codegen fix for boxed out parameters
Last modified: 2017-12-03 20:02:12 UTC
Created attachment 292195 [details] [review] Git-format patch against glib-2-42 When using gdbus-codegen to produce generated code for a method like this: <method name="GetPrinters"> <arg name="printers" direction="out" type="ao"/> </method> it produces output like this: /** * pd_manager_call_get_printers_sync: * @proxy: A #PdManagerProxy. * @out_printers: (out): Return location for return parameter or %NULL to ignore. * @cancellable: (allow-none): A #GCancellable or %NULL. * @error: Return location for error or %NULL. Because out_printers is not annotated as being an array, the g-ir-scanner produces this GIR output: <parameter name="out_printers" direction="out" caller-allocates="0" transfer-ownership="full"> <type name="utf8" c:type="gchar***"/> </parameter> ...which is nonsense. The fix is to include an "(array zero-terminated=1)" annotation. Patch attached. Original discovery: https://github.com/hughsie/printerd/issues/40 Patch against glib-2-42 attached.
Created attachment 363674 [details] [review] codegen: fix array out-param annotations When using gdbus-codegen to produce generated code for a method with an out parameter with a signature like 'as', make sure to include an "(array)" annotation for that parameter. (Reworked by Philip Withnall to improve code formatting.)
Created attachment 363675 [details] [review] gdbusutils: Fix a memory leak in g_dbus_gvalue_to_gvariant() g_variant_get_normal_form() doesn’t necessarily return a floating GVariant, so we have to take, rather than sink, the ref. This fixes a lot of leaks with gdbus-codegen-generated code. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Created attachment 363676 [details] [review] tests: Fix some minor memory leaks in gdbus-test-codegen It’s not entirely leak-free, but it’s better than before. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Created attachment 363677 [details] [review] gvariant: Clarify return docs for g_variant_get_normal_form() Clarify that the return value may be floating, or may not be (depends on whether the input @value was in normal form). Signed-off-by: Philip Withnall <withnall@endlessm.com>
Created attachment 363678 [details] [review] gobject: Add missing annotations to GValue variant methods They were missing some (nullable) and (transfer) annotations. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Review of attachment 363674 [details] [review]: :+1:
Review of attachment 363675 [details] [review]: Nice catch
Review of attachment 363676 [details] [review]: Okay
Review of attachment 363677 [details] [review]: ::: glib/gvariant.c @@ +5856,3 @@ * output is definitely in normal form. * + * The return value may be floating. This is kind of unhelpful; does it depend on the phase of the Moon or the alignment between planets? If the GVariant is already a normal form, it'll return a reference to that GVariant, otherwise it'll ref_sink it. I think this should be the documented behaviour of the function, given that we cannot change it in a way that doesn't break existing code. To be fair, I find this function vaguely confusing.
Review of attachment 363678 [details] [review]: Okay
Attachment 363674 [details] pushed as d35d9b7 - codegen: fix array out-param annotations Attachment 363675 [details] pushed as 37d9b0c - gdbusutils: Fix a memory leak in g_dbus_gvalue_to_gvariant() Attachment 363676 [details] pushed as a2a4a10 - tests: Fix some minor memory leaks in gdbus-test-codegen Attachment 363678 [details] pushed as aca410c - gobject: Add missing annotations to GValue variant methods
Created attachment 364566 [details] [review] gvariant: Clarify return docs for g_variant_get_normal_form() Clarify that the return value may be floating, or may not be (depends on whether the input @value was in normal form). Signed-off-by: Philip Withnall <withnall@endlessm.com>
Review of attachment 364566 [details] [review]: ACK-by: me
Ta. Attachment 364566 [details] pushed as e45f99e - gvariant: Clarify return docs for g_variant_get_normal_form()