From 9b827e5674df255961de97e169f5a299be9afea1 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Mon, 2 Sep 2019 06:54:37 +0100 Subject: [PATCH 1/2] gdbus-codegen: process C.UnixFD annotation in one place This will make it simpler to enable this behaviour based on the method signature. https://gitlab.gnome.org/GNOME/glib/issues/1726 --- gio/gdbus-2.0/codegen/codegen.py | 75 +++++++++++------------------- gio/gdbus-2.0/codegen/dbustypes.py | 4 ++ gio/tests/gdbus-test-codegen.c | 26 +++++++++++ 3 files changed, 57 insertions(+), 48 deletions(-) diff --git a/gio/gdbus-2.0/codegen/codegen.py b/gio/gdbus-2.0/codegen/codegen.py index aa1280f59..55cd7bb34 100644 --- a/gio/gdbus-2.0/codegen/codegen.py +++ b/gio/gdbus-2.0/codegen/codegen.py @@ -112,14 +112,11 @@ class HeaderCodeGenerator: if len(i.methods) > 0: self.outfile.write('\n') for m in i.methods: - unix_fd = False - if utils.lookup_annotation(m.annotations, 'org.gtk.GDBus.C.UnixFD'): - unix_fd = True key = (m.since, '_method_%s'%m.name_lower) value = ' gboolean (*handle_%s) (\n'%(m.name_lower) value += ' %s *object,\n'%(i.camel_name) value += ' GDBusMethodInvocation *invocation'%() - if unix_fd: + if m.unix_fd: value += ',\n GUnixFDList *fd_list' for a in m.in_args: value += ',\n %sarg_%s'%(a.ctype_in, a.name) @@ -176,15 +173,12 @@ class HeaderCodeGenerator: self.outfile.write('\n') self.outfile.write('/* D-Bus method call completion functions: */\n') for m in i.methods: - unix_fd = False - if utils.lookup_annotation(m.annotations, 'org.gtk.GDBus.C.UnixFD'): - unix_fd = True if m.deprecated: self.outfile.write('G_GNUC_DEPRECATED ') self.outfile.write('void %s_complete_%s (\n' ' %s *object,\n' ' GDBusMethodInvocation *invocation'%(i.name_lower, m.name_lower, i.camel_name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList *fd_list') for a in m.out_args: self.outfile.write(',\n %s%s'%(a.ctype_in, a.name)) @@ -212,9 +206,6 @@ class HeaderCodeGenerator: self.outfile.write('\n') self.outfile.write('/* D-Bus method calls: */\n') for m in i.methods: - unix_fd = False - if utils.lookup_annotation(m.annotations, 'org.gtk.GDBus.C.UnixFD'): - unix_fd = True # async begin if m.deprecated: self.outfile.write('G_GNUC_DEPRECATED ') @@ -222,7 +213,7 @@ class HeaderCodeGenerator: ' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name)) for a in m.in_args: self.outfile.write(',\n %sarg_%s'%(a.ctype_in, a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList *fd_list') self.outfile.write(',\n' ' GCancellable *cancellable,\n' @@ -236,7 +227,7 @@ class HeaderCodeGenerator: ' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name)) for a in m.out_args: self.outfile.write(',\n %sout_%s'%(a.ctype_out, a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList **out_fd_list') self.outfile.write(',\n' ' GAsyncResult *res,\n' @@ -249,11 +240,11 @@ class HeaderCodeGenerator: ' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name)) for a in m.in_args: self.outfile.write(',\n %sarg_%s'%(a.ctype_in, a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList *fd_list') for a in m.out_args: self.outfile.write(',\n %sout_%s'%(a.ctype_out, a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList **out_fd_list') self.outfile.write(',\n' ' GCancellable *cancellable,\n' @@ -1153,9 +1144,6 @@ class CodeGenerator: if len(i.methods) > 0: for m in i.methods: - unix_fd = False - if utils.lookup_annotation(m.annotations, 'org.gtk.GDBus.C.UnixFD'): - unix_fd = True self.generate_args('_%s_method_info_%s_IN_ARG'%(i.name_lower, m.name_lower), m.in_args) self.generate_args('_%s_method_info_%s_OUT_ARG'%(i.name_lower, m.name_lower), m.out_args) @@ -1181,7 +1169,7 @@ class CodeGenerator: self.outfile.write(' },\n' ' "handle-%s",\n' ' %s\n' - %(m.name_hyphen, 'TRUE' if unix_fd else 'FALSE')) + %(m.name_hyphen, 'TRUE' if m.unix_fd else 'FALSE')) self.outfile.write('};\n' '\n') @@ -1399,16 +1387,13 @@ class CodeGenerator: if len(i.methods) > 0: self.outfile.write(' /* GObject signals for incoming D-Bus method calls: */\n') for m in i.methods: - unix_fd = False - if utils.lookup_annotation(m.annotations, 'org.gtk.GDBus.C.UnixFD'): - unix_fd = True self.outfile.write(self.docbook_gen.expand( ' /**\n' ' * %s::handle-%s:\n' ' * @object: A #%s.\n' ' * @invocation: A #GDBusMethodInvocation.\n' %(i.camel_name, m.name_hyphen, i.camel_name), False)) - if unix_fd: + if m.unix_fd: self.outfile.write(' * @fd_list: (nullable): A #GUnixFDList or %NULL.\n') for a in m.in_args: self.outfile.write(' * @arg_%s: Argument passed by remote caller.\n'%(a.name)) @@ -1421,7 +1406,7 @@ class CodeGenerator: ' * Returns: %%TRUE if the invocation was handled, %%FALSE to let other signal handlers run.\n' %(i.name, m.name, i.name_lower, m.name_lower), False)) self.write_gtkdoc_deprecated_and_since_and_close(m, self.outfile, 2) - if unix_fd: + if m.unix_fd: extra_args = 2 else: extra_args = 1 @@ -1436,7 +1421,7 @@ class CodeGenerator: ' %d,\n' ' G_TYPE_DBUS_METHOD_INVOCATION' %(m.name_hyphen, i.camel_name, m.name_lower, len(m.in_args) + extra_args)) - if unix_fd: + if m.unix_fd: self.outfile.write(', G_TYPE_UNIX_FD_LIST') for a in m.in_args: self.outfile.write(', %s'%(a.gtype)) @@ -1662,9 +1647,6 @@ class CodeGenerator: def generate_method_calls(self, i): for m in i.methods: - unix_fd = False - if utils.lookup_annotation(m.annotations, 'org.gtk.GDBus.C.UnixFD'): - unix_fd = True # async begin self.outfile.write('/**\n' ' * %s_call_%s:\n' @@ -1672,7 +1654,7 @@ class CodeGenerator: %(i.name_lower, m.name_lower, i.camel_name)) for a in m.in_args: self.outfile.write(' * @arg_%s: Argument to pass with the method invocation.\n'%(a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(' * @fd_list: (nullable): A #GUnixFDList or %NULL.\n') self.outfile.write(self.docbook_gen.expand( ' * @cancellable: (nullable): A #GCancellable or %%NULL.\n' @@ -1691,14 +1673,14 @@ class CodeGenerator: ' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name)) for a in m.in_args: self.outfile.write(',\n %sarg_%s'%(a.ctype_in, a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList *fd_list') self.outfile.write(',\n' ' GCancellable *cancellable,\n' ' GAsyncReadyCallback callback,\n' ' gpointer user_data)\n' '{\n') - if unix_fd: + if m.unix_fd: self.outfile.write(' g_dbus_proxy_call_with_unix_fd_list (G_DBUS_PROXY (proxy),\n') else: self.outfile.write(' g_dbus_proxy_call (G_DBUS_PROXY (proxy),\n') @@ -1712,7 +1694,7 @@ class CodeGenerator: self.outfile.write('),\n' ' G_DBUS_CALL_FLAGS_NONE,\n' ' -1,\n') - if unix_fd: + if m.unix_fd: self.outfile.write(' fd_list,\n') self.outfile.write(' cancellable,\n' ' callback,\n' @@ -1726,7 +1708,7 @@ class CodeGenerator: %(i.name_lower, m.name_lower, i.camel_name)) for a in m.out_args: self.outfile.write(' * @out_%s: (out) (optional)%s: Return location for return parameter or %%NULL to ignore.\n'%(a.name, ' ' + a.array_annotation if a.array_annotation else '')) - if unix_fd: + if m.unix_fd: self.outfile.write(' * @out_fd_list: (out) (optional): Return location for a #GUnixFDList or %NULL to ignore.\n') self.outfile.write(self.docbook_gen.expand( ' * @res: The #GAsyncResult obtained from the #GAsyncReadyCallback passed to %s_call_%s().\n' @@ -1742,14 +1724,14 @@ class CodeGenerator: ' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name)) for a in m.out_args: self.outfile.write(',\n %sout_%s'%(a.ctype_out, a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList **out_fd_list') self.outfile.write(',\n' ' GAsyncResult *res,\n' ' GError **error)\n' '{\n' ' GVariant *_ret;\n') - if unix_fd: + if m.unix_fd: self.outfile.write(' _ret = g_dbus_proxy_call_with_unix_fd_list_finish (G_DBUS_PROXY (proxy), out_fd_list, res, error);\n') else: self.outfile.write(' _ret = g_dbus_proxy_call_finish (G_DBUS_PROXY (proxy), res, error);\n') @@ -1777,11 +1759,11 @@ class CodeGenerator: %(i.name_lower, m.name_lower, i.camel_name)) for a in m.in_args: self.outfile.write(' * @arg_%s: Argument to pass with the method invocation.\n'%(a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(' * @fd_list: (nullable): A #GUnixFDList or %NULL.\n') for a in m.out_args: self.outfile.write(' * @out_%s: (out) (optional)%s: Return location for return parameter or %%NULL to ignore.\n'%(a.name, ' ' + a.array_annotation if a.array_annotation else '')) - if unix_fd: + if m.unix_fd: self.outfile.write(' * @out_fd_list: (out): Return location for a #GUnixFDList or %NULL.\n') self.outfile.write(self.docbook_gen.expand( ' * @cancellable: (nullable): A #GCancellable or %%NULL.\n' @@ -1799,18 +1781,18 @@ class CodeGenerator: ' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name)) for a in m.in_args: self.outfile.write(',\n %sarg_%s'%(a.ctype_in, a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList *fd_list') for a in m.out_args: self.outfile.write(',\n %sout_%s'%(a.ctype_out, a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList **out_fd_list') self.outfile.write(',\n' ' GCancellable *cancellable,\n' ' GError **error)\n' '{\n' ' GVariant *_ret;\n') - if unix_fd: + if m.unix_fd: self.outfile.write(' _ret = g_dbus_proxy_call_with_unix_fd_list_sync (G_DBUS_PROXY (proxy),\n') else: self.outfile.write(' _ret = g_dbus_proxy_call_sync (G_DBUS_PROXY (proxy),\n') @@ -1824,7 +1806,7 @@ class CodeGenerator: self.outfile.write('),\n' ' G_DBUS_CALL_FLAGS_NONE,\n' ' -1,\n') - if unix_fd: + if m.unix_fd: self.outfile.write(' fd_list,\n' ' out_fd_list,\n') self.outfile.write(' cancellable,\n' @@ -1849,15 +1831,12 @@ class CodeGenerator: def generate_method_completers(self, i): for m in i.methods: - unix_fd = False - if utils.lookup_annotation(m.annotations, 'org.gtk.GDBus.C.UnixFD'): - unix_fd = True self.outfile.write('/**\n' ' * %s_complete_%s:\n' ' * @object: A #%s.\n' ' * @invocation: (transfer full): A #GDBusMethodInvocation.\n' %(i.name_lower, m.name_lower, i.camel_name)) - if unix_fd: + if m.unix_fd: self.outfile.write(' * @fd_list: (nullable): A #GUnixFDList or %NULL.\n') for a in m.out_args: self.outfile.write(' * @%s: Parameter to return.\n'%(a.name)) @@ -1872,14 +1851,14 @@ class CodeGenerator: '%s_complete_%s (\n' ' %s *object,\n' ' GDBusMethodInvocation *invocation'%(i.name_lower, m.name_lower, i.camel_name)) - if unix_fd: + if m.unix_fd: self.outfile.write(',\n GUnixFDList *fd_list') for a in m.out_args: self.outfile.write(',\n %s%s'%(a.ctype_in, a.name)) self.outfile.write(')\n' '{\n') - if unix_fd: + if m.unix_fd: self.outfile.write(' g_dbus_method_invocation_return_value_with_unix_fd_list (invocation,\n' ' g_variant_new ("(') else: @@ -1890,7 +1869,7 @@ class CodeGenerator: self.outfile.write(')"') for a in m.out_args: self.outfile.write(',\n %s'%(a.name)) - if unix_fd: + if m.unix_fd: self.outfile.write('),\n fd_list);\n') else: self.outfile.write('));\n') diff --git a/gio/gdbus-2.0/codegen/dbustypes.py b/gio/gdbus-2.0/codegen/dbustypes.py index 60c53c4be..16364f9b7 100644 --- a/gio/gdbus-2.0/codegen/dbustypes.py +++ b/gio/gdbus-2.0/codegen/dbustypes.py @@ -260,6 +260,7 @@ class Method: self.doc_string = '' self.since = '' self.deprecated = False + self.unix_fd = False def post_process(self, interface_prefix, cns, cns_upper, cns_lower, containing_iface): if len(self.doc_string) == 0: @@ -291,6 +292,9 @@ class Method: if utils.lookup_annotation(self.annotations, 'org.freedesktop.DBus.Deprecated') == 'true': self.deprecated = True + if utils.lookup_annotation(self.annotations, 'org.gtk.GDBus.C.UnixFD'): + self.unix_fd = True + for a in self.annotations: a.post_process(interface_prefix, cns, cns_upper, cns_lower, self) diff --git a/gio/tests/gdbus-test-codegen.c b/gio/tests/gdbus-test-codegen.c index 985cb1095..241cc529c 100644 --- a/gio/tests/gdbus-test-codegen.c +++ b/gio/tests/gdbus-test-codegen.c @@ -2588,6 +2588,31 @@ test_standalone_interface_info (void) g_clear_object (&skel); } +/* ---------------------------------------------------------------------------------------------------- */ +static gboolean +handle_hello_fd (FooiGenFDPassing *object, + GDBusMethodInvocation *invocation, + GUnixFDList *fd_list, + const gchar *arg_greeting) +{ + foo_igen_fdpassing_complete_hello_fd (object, invocation, fd_list, arg_greeting); + return TRUE; +} + +/* Test that generated code for methods includes GUnixFDList arguments if + * the method is explicitly annotated as C.UnixFD. + */ +static void +test_unix_fd_list (void) +{ + FooiGenFDPassingIface iface; + + /* This method is explicitly annotated. */ + iface.handle_hello_fd = handle_hello_fd; + + (void) iface; +} + /* ---------------------------------------------------------------------------------------------------- */ int @@ -2603,6 +2628,7 @@ main (int argc, g_test_add_func ("/gdbus/codegen/autocleanups", test_autocleanups); g_test_add_func ("/gdbus/codegen/deprecations", test_deprecations); g_test_add_func ("/gdbus/codegen/standalone-interface-info", test_standalone_interface_info); + g_test_add_func ("/gdbus/codegen/unix-fd-list", test_unix_fd_list); return session_bus_run (); } From 4aba03562bc1526a1baf70ad068a9395ec64bf64 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Mon, 2 Sep 2019 06:56:08 +0100 Subject: [PATCH 2/2] gdbus-codegen: emit GUnixFDLists if an arg has type 'h' Previously, if a method was not annotated with org.gtk.GDBus.C.UnixFD then the generated code would never contain GUnixFDList parameters, even if the method has 'h' (file descriptor) parameters. However, in this case, the generated code is essentially useless: the method cannot be called or handled except in degenerate cases where the file descriptors are missing or ignored. Check the argument types for 'h', and if present, generate code as if org.gtk.GDBus.C.UnixFD annotation were specified. This change will break any existing code which refers to the (useless) wrappers for such methods. The workaround for such code is to add the org.gtk.GDBus.C.UnixFD annotation, which will cause the same generated code to be emitted before and after this change. If this is found to cause widespread problems, we can explore a different approach (perhaps emitting a warning from the code generator, or annotating the symbols as deprecated). https://gitlab.gnome.org/GNOME/glib/issues/1726 --- gio/gdbus-2.0/codegen/dbustypes.py | 4 ++++ gio/tests/gdbus-test-codegen.c | 34 ++++++++++++++++++++++++++++-- gio/tests/test-codegen.xml | 9 ++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/gio/gdbus-2.0/codegen/dbustypes.py b/gio/gdbus-2.0/codegen/dbustypes.py index 16364f9b7..498ea668b 100644 --- a/gio/gdbus-2.0/codegen/dbustypes.py +++ b/gio/gdbus-2.0/codegen/dbustypes.py @@ -284,10 +284,14 @@ class Method: for a in self.in_args: a.post_process(interface_prefix, cns, cns_upper, cns_lower, arg_count) arg_count += 1 + if 'h' in a.signature: + self.unix_fd = True for a in self.out_args: a.post_process(interface_prefix, cns, cns_upper, cns_lower, arg_count) arg_count += 1 + if 'h' in a.signature: + self.unix_fd = True if utils.lookup_annotation(self.annotations, 'org.freedesktop.DBus.Deprecated') == 'true': self.deprecated = True diff --git a/gio/tests/gdbus-test-codegen.c b/gio/tests/gdbus-test-codegen.c index 241cc529c..ba0f6ed61 100644 --- a/gio/tests/gdbus-test-codegen.c +++ b/gio/tests/gdbus-test-codegen.c @@ -2599,16 +2599,46 @@ handle_hello_fd (FooiGenFDPassing *object, return TRUE; } -/* Test that generated code for methods includes GUnixFDList arguments if - * the method is explicitly annotated as C.UnixFD. +static gboolean +handle_no_annotation (FooiGenFDPassing *object, + GDBusMethodInvocation *invocation, + GUnixFDList *fd_list, + GVariant *arg_greeting, + const gchar *arg_greeting_locale) +{ + foo_igen_fdpassing_complete_no_annotation (object, invocation, fd_list, arg_greeting, arg_greeting_locale); + return TRUE; +} + +static gboolean +handle_no_annotation_nested (FooiGenFDPassing *object, + GDBusMethodInvocation *invocation, + GUnixFDList *fd_list, + GVariant *arg_files) +{ + foo_igen_fdpassing_complete_no_annotation_nested (object, invocation, fd_list); + return TRUE; +} + +/* Test that generated code for methods includes GUnixFDList arguments if: + * - the method is explicitly annotated as C.UnixFD; or + * - the method signature contains the type 'h' */ static void test_unix_fd_list (void) { FooiGenFDPassingIface iface; + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1726"); + /* This method is explicitly annotated. */ iface.handle_hello_fd = handle_hello_fd; + /* This one is not, but it's got an in and out 'h' parameter so should + * automatically grow GUnixFDList arguments. + */ + iface.handle_no_annotation = handle_no_annotation; + /* This method has an 'h' inside a complex type. */ + iface.handle_no_annotation_nested = handle_no_annotation_nested; (void) iface; } diff --git a/gio/tests/test-codegen.xml b/gio/tests/test-codegen.xml index 39d8769c7..3090cad4a 100644 --- a/gio/tests/test-codegen.xml +++ b/gio/tests/test-codegen.xml @@ -481,6 +481,15 @@ + + + + + + + + +