From 1b345e411fa25c774d3429d9083a4fd5e5ac3ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 02:51:44 +0200 Subject: [PATCH 01/11] meson: Add all the gdbus-codegen based tests to the proper suite So it's easier to run them all with --suite=gdbus-codegen --- gio/tests/meson.build | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gio/tests/meson.build b/gio/tests/meson.build index 2ff34a5d0..c1240b527 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -170,7 +170,10 @@ test_extra_programs = { python_tests = { # FIXME: https://gitlab.gnome.org/GNOME/glib/-/issues/2764 - 'codegen.py' : { 'can_fail' : host_system == 'freebsd' }, + 'codegen.py' : { + 'can_fail' : host_system == 'freebsd', + 'suite': ['gdbus-codegen'], + }, } test_env = environment() @@ -458,6 +461,7 @@ if host_machine.system() != 'windows' 'gdbus-test-codegen' : { 'extra_sources' : [extra_sources, gdbus_test_codegen_generated, gdbus_test_codegen_generated_interface_info], 'c_args' : ['-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_32'], + 'suite': ['gdbus-codegen'], }, 'gdbus-threading' : { 'extra_sources' : extra_sources, @@ -476,11 +480,13 @@ if host_machine.system() != 'windows' 'extra_sources' : [extra_sources, gdbus_test_codegen_generated, gdbus_test_codegen_generated_interface_info], 'c_args' : ['-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_36', '-DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_36'], + 'suite': ['gdbus-codegen'], }, 'gdbus-test-codegen-min-required-2-64' : { 'source' : 'gdbus-test-codegen.c', 'extra_sources' : [extra_sources, gdbus_test_codegen_generated_min_required_2_64, gdbus_test_codegen_generated_interface_info], 'c_args' : ['-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_64'], + 'suite': ['gdbus-codegen'], }, 'gapplication' : { 'extra_sources' : extra_sources, @@ -1045,7 +1051,7 @@ endforeach foreach test_name, extra_args : python_tests depends = [extra_args.get('depends', [])] - suite = ['gio', 'no-valgrind'] + suite = ['gio', 'no-valgrind'] + extra_args.get('suite', []) if extra_args.get('can_fail', false) suite += 'failing' From 4b05e06e1c1d5b865f5ec87cb14dac1a1929f667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 16:25:31 +0200 Subject: [PATCH 02/11] gdbus-codegen: Address more black cleanups --- gio/gdbus-2.0/codegen/codegen.py | 3 +-- gio/gdbus-2.0/codegen/codegen_main.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/gio/gdbus-2.0/codegen/codegen.py b/gio/gdbus-2.0/codegen/codegen.py index 6fa163322..0616c3492 100644 --- a/gio/gdbus-2.0/codegen/codegen.py +++ b/gio/gdbus-2.0/codegen/codegen.py @@ -1138,7 +1138,7 @@ class InterfaceInfoBodyCodeGenerator: "const %s * const %s[] =\n" % (element_type, array_name_lower) ) self.outfile.write("{\n") - for (_, name) in elements: + for _, name in elements: self.outfile.write(" &%s,\n" % name) self.outfile.write(" NULL,\n") self.outfile.write("};\n") @@ -2397,7 +2397,6 @@ class CodeGenerator: self.outfile.write("}\n") self.outfile.write("\n") if p.arg.free_func is not None: - self.outfile.write( self.docbook_gen.expand( "/**\n" diff --git a/gio/gdbus-2.0/codegen/codegen_main.py b/gio/gdbus-2.0/codegen/codegen_main.py index 69a264c81..c0f44d50a 100644 --- a/gio/gdbus-2.0/codegen/codegen_main.py +++ b/gio/gdbus-2.0/codegen/codegen_main.py @@ -119,7 +119,7 @@ def apply_annotation(iface_list, iface, method, signal, prop, arg, key, value): def apply_annotations(iface_list, annotation_list): # apply annotations given on the command line - for (what, key, value) in annotation_list: + for what, key, value in annotation_list: pos = what.find("::") if pos != -1: # signal From 51c023f18906fac9f34569e6e34f414206291d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 16:25:01 +0200 Subject: [PATCH 03/11] tests/codegen: Add checks for signal emission enums These are checked to be correct by code compilation, but let's ensure we also have some kind of forma check on the expected syntax. --- gio/tests/codegen.py | 132 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/gio/tests/codegen.py b/gio/tests/codegen.py index d2d8c7226..397e16f36 100644 --- a/gio/tests/codegen.py +++ b/gio/tests/codegen.py @@ -60,6 +60,27 @@ class TestCodegen(unittest.TestCase): # Track the cwd, we want to back out to that to clean up our tempdir cwd = "" + ARGUMENTS_TYPES = { + "b": {}, + "y": {}, + "n": {}, + "q": {}, + "i": {}, + "u": {}, + "x": {}, + "t": {}, + "d": {}, + "s": {}, + "o": {}, + "g": {}, + "h": {}, + "ay": {}, + "as": {}, + "ao": {}, + "aay": {}, + "asv": {"variant_type": "a{sv}"}, + } + def setUp(self): self.timeout_seconds = 6 # seconds per test self.tmpdir = tempfile.TemporaryDirectory() @@ -665,6 +686,117 @@ G_END_DECLS self.assertEqual(result.out.strip().count("GDBusCallFlags call_flags,"), 2) self.assertEqual(result.out.strip().count("gint timeout_msec,"), 2) + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_signal_id_simple_signal(self): + """Test that signals IDs are used to emit signals""" + interface_xml = """ + + + + + + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_signal_emit_by_name ("), 0) + + for iface in ["USEFUL_INTERFACE", "OTHER_IFACE"]: + enum_name = f"_ORG_PROJECT_{iface}_SIGNALS" + enum_item = f"_ORG_PROJECT_{iface}_SIMPLE_SIGNAL" + self.assertIs(stripped_out.count(f"{enum_item},"), 1) + self.assertIs(stripped_out.count(f"{enum_name}[{enum_item}] ="), 1) + self.assertIs( + stripped_out.count( + f" g_signal_emit (object, {enum_name}[{enum_item}], 0);" + ), + 1, + ) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_signal_id_multiple_signals_types(self): + """Test that signals IDs are used to emit signals for all types""" + + signal_template = "" + generated_signals = [ + signal_template.format( + f"SingleArgSignal{t.upper()}", f"an_{t}", props.get("variant_type", t) + ) + for t, props in self.ARGUMENTS_TYPES.items() + ] + + interface_xml = f""" + + + + {''.join(generated_signals)} + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_signal_emit_by_name ("), 0) + + iface = "SIGNALING_IFACE" + for t in self.ARGUMENTS_TYPES.keys(): + enum_name = f"_ORG_PROJECT_{iface}_SIGNALS" + enum_item = f"_ORG_PROJECT_{iface}_SINGLE_ARG_SIGNAL_{t.upper()}" + self.assertIs(stripped_out.count(f"{enum_item},"), 1) + self.assertIs(stripped_out.count(f"{enum_name}[{enum_item}] ="), 1) + self.assertIs( + stripped_out.count( + f" g_signal_emit (object, {enum_name}[{enum_item}], 0, arg_an_{t});" + ), + 1, + ) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_signal_id_multiple_signal_args_types(self): + """Test that signals IDs are used to emit signals for all types""" + + generated_args = [ + f"\n" + for t, props in self.ARGUMENTS_TYPES.items() + ] + + interface_xml = f""" + + + + {''.join(generated_args)} + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_signal_emit_by_name ("), 0) + + iface = "SIGNALING_IFACE" + enum_name = f"_ORG_PROJECT_{iface}_SIGNALS" + enum_item = f"_ORG_PROJECT_{iface}_SIGNAL_WITH_MANY_ARGS" + self.assertIs(stripped_out.count(f"{enum_item},"), 1) + self.assertIs(stripped_out.count(f"{enum_name}[{enum_item}] ="), 1) + + args = ", ".join([f"arg_an_{t}" for t in self.ARGUMENTS_TYPES.keys()]) + self.assertIs( + stripped_out.count( + f" g_signal_emit (object, {enum_name}[{enum_item}], 0, {args});" + ), + 1, + ) + def test_generate_valid_docbook(self): """Test the basic functionality of the docbook generator.""" xml_contents = """ From 1cd5c5678a7c51274f2cccdd4a7d28a5f917782b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 02:52:11 +0200 Subject: [PATCH 04/11] gdbus-codegen: Generate signal marshallers for each interface signal We relied on g_cclosure_marshal_generic() to easily generate signal marshallers, but this relies on inspecting each parameter type with ffi and this implies a performance hit, other than breaking the stack-frame unwinder used by Linux perf and so by sysprof. Given that we know the types we work on, it's easy enough to generate the marshallers ourself. Helps with: https://gitlab.gnome.org/GNOME/glib/-/issues/3028 --- gio/gdbus-2.0/codegen/codegen.py | 74 +++++++++++++++- gio/gdbus-2.0/codegen/utils.py | 4 + gio/tests/codegen.py | 142 +++++++++++++++++++++++++++---- 3 files changed, 201 insertions(+), 19 deletions(-) diff --git a/gio/gdbus-2.0/codegen/codegen.py b/gio/gdbus-2.0/codegen/codegen.py index 0616c3492..a5fd98772 100644 --- a/gio/gdbus-2.0/codegen/codegen.py +++ b/gio/gdbus-2.0/codegen/codegen.py @@ -2187,7 +2187,7 @@ class CodeGenerator: " G_STRUCT_OFFSET (%sIface, %s),\n" " NULL,\n" # accumulator " NULL,\n" # accu_data - " g_cclosure_marshal_generic,\n" + f" {i.name_lower}_signal_marshal_{s.name_lower},\n" " G_TYPE_NONE,\n" " %d" % (s.name_hyphen, i.camel_name, s.name_lower, len(s.args)) @@ -2525,6 +2525,77 @@ class CodeGenerator: # --------------------------------------------------------------------------------------------------- + def generate_marshaller(self, func_name, in_args): + self.generate_marshaller_declaration(func_name) + self.outfile.write("{\n") + self.generate_marshaller_body(func_name, in_args) + self.outfile.write("}\n" "\n") + + def generate_marshaller_declaration(self, func_name): + self.outfile.write( + "static void\n" + f"{func_name} (\n" + " GClosure *closure,\n" + " GValue *return_value G_GNUC_UNUSED,\n" + " unsigned int n_param_values,\n" + " const GValue *param_values,\n" + " void *invocation_hint G_GNUC_UNUSED,\n" + " void *marshal_data)\n" + ) + + def generate_marshaller_body(self, func_name, in_args=[]): + marshal_func_type = f"{utils.uscore_to_camel_case(func_name)}Func" + self.outfile.write( + f" typedef void (*{marshal_func_type})\n" + " (void *data1,\n" + + "".join([f" {a.ctype_in}arg_{a.name},\n" for a in in_args]) + + " void *data2);\n" + f" {marshal_func_type} callback;\n" + " GCClosure *cc = (GCClosure*) closure;\n" + f" void *data1, *data2;\n" + "\n" + f" g_return_if_fail (n_param_values == {len(in_args) + 1});\n" + "\n" + " if (G_CCLOSURE_SWAP_DATA (closure))\n" + " {\n" + " data1 = closure->data;\n" + " data2 = g_value_peek_pointer (param_values + 0);\n" + " }\n" + " else\n" + " {\n" + " data1 = g_value_peek_pointer (param_values + 0);\n" + " data2 = closure->data;\n" + " }\n" + "\n" + f" callback = ({marshal_func_type})\n" + " (marshal_data ? marshal_data : cc->callback);\n" + "\n" + " callback (data1,\n" + + "".join( + [ + f" {in_args[i].gvalue_get} (param_values + {i+1}),\n" + for i in range(len(in_args)) + ] + ) + + " data2);\n" + ) + + def generate_marshaller_for_type(self, i, t): + assert isinstance(t, dbustypes.Signal) + + kind_uscore = utils.camel_case_to_uscore(t.__class__.__name__.lower()) + + self.generate_marshaller( + func_name=f"{i.name_lower}_{kind_uscore}_marshal_{t.name_lower}", + in_args=t.args, + ) + + def generate_signal_marshallers(self, i): + for s in i.signals: + self.generate_marshaller_for_type(i, s) + + # --------------------------------------------------------------------------------------------------- + def generate_method_calls(self, i): for m in i.methods: # async begin @@ -5249,6 +5320,7 @@ class CodeGenerator: self.generate_interface_intro(i) self.generate_signals_enum_for_interface(i) self.generate_introspection_for_interface(i) + self.generate_signal_marshallers(i) self.generate_interface(i) self.generate_property_accessors(i) self.generate_signal_emitters(i) diff --git a/gio/gdbus-2.0/codegen/utils.py b/gio/gdbus-2.0/codegen/utils.py index 95559d37c..02046108d 100644 --- a/gio/gdbus-2.0/codegen/utils.py +++ b/gio/gdbus-2.0/codegen/utils.py @@ -114,6 +114,10 @@ def camel_case_to_uscore(s): return ret +def uscore_to_camel_case(s): + return "".join([s[0].upper() + s[1:].lower() if s else "_" for s in s.split("_")]) + + def is_ugly_case(s): if s and s.find("_") > 0: return True diff --git a/gio/tests/codegen.py b/gio/tests/codegen.py index 397e16f36..d591df192 100644 --- a/gio/tests/codegen.py +++ b/gio/tests/codegen.py @@ -61,24 +61,24 @@ class TestCodegen(unittest.TestCase): cwd = "" ARGUMENTS_TYPES = { - "b": {}, - "y": {}, - "n": {}, - "q": {}, - "i": {}, - "u": {}, - "x": {}, - "t": {}, - "d": {}, - "s": {}, - "o": {}, - "g": {}, - "h": {}, - "ay": {}, - "as": {}, - "ao": {}, - "aay": {}, - "asv": {"variant_type": "a{sv}"}, + "b": {"value_type": "boolean"}, + "y": {"value_type": "uchar"}, + "n": {"value_type": "int"}, + "q": {"value_type": "uint"}, + "i": {"value_type": "int"}, + "u": {"value_type": "uint"}, + "x": {"value_type": "int64"}, + "t": {"value_type": "uint64"}, + "d": {"value_type": "double"}, + "s": {"value_type": "string"}, + "o": {"value_type": "string"}, + "g": {"value_type": "string"}, + "h": {"value_type": "variant"}, + "ay": {"value_type": "string"}, + "as": {"value_type": "boxed"}, + "ao": {"value_type": "boxed"}, + "aay": {"value_type": "boxed"}, + "asv": {"value_type": "variant", "variant_type": "a{sv}"}, } def setUp(self): @@ -797,6 +797,112 @@ G_END_DECLS 1, ) + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_signals_marshaller_simple_signal(self): + """Test that signals marshaller is generated for simple signal""" + interface_xml = """ + + + + + + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = "org_project_signaling_iface_signal_marshal_simple_signal" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + func_name = "org_project_other_signaling_iface_signal_marshal_simple_signal" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_signals_marshaller_single_typed_args(self): + """Test that signals marshaller is generated for each known type""" + for t, props in self.ARGUMENTS_TYPES.items(): + camel_type = t[0].upper() + t[1:] + interface_xml = f""" + + + + + + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertEqual(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = ( + f"org_project_signaling_iface_signal_marshal_single_arg_signal_{t}" + ) + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + self.assertIs( + stripped_out.count( + f"g_value_get_{props['value_type']} (param_values + 1)" + ), + 1, + ) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_signals_marshallers_multiple_args(self): + """Test that signals marshallers are generated""" + generated_args = [ + f"\n" + for t, props in self.ARGUMENTS_TYPES.items() + ] + + interface_xml = f""" + + + + + {''.join(generated_args)} + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = f"org_project_signaling_iface_signal_marshal_simple_signal" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + func_name = f"org_project_signaling_iface_signal_marshal_signal_with_many_args" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + # Check access to MultipleArgsSignal arguments + index = 1 + for props in self.ARGUMENTS_TYPES.values(): + self.assertIs( + stripped_out.count( + f"g_value_get_{props['value_type']} (param_values + {index})" + ), + 1, + ) + index += 1 + def test_generate_valid_docbook(self): """Test the basic functionality of the docbook generator.""" xml_contents = """ From 7b750972e8a1951f2037e8cb004ae508b8a3e33c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 16:26:09 +0200 Subject: [PATCH 05/11] meson/gio: Increase timeout for the codegen python tests It's adding more stuff, let's ensure it wont timeout for no reason in some platforms. As per this mark also it as 'slow' --- gio/tests/meson.build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gio/tests/meson.build b/gio/tests/meson.build index c1240b527..8d9c87c20 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -172,7 +172,8 @@ python_tests = { # FIXME: https://gitlab.gnome.org/GNOME/glib/-/issues/2764 'codegen.py' : { 'can_fail' : host_system == 'freebsd', - 'suite': ['gdbus-codegen'], + 'suite': ['gdbus-codegen', 'slow'], + 'timeout': 90, }, } @@ -1068,6 +1069,7 @@ foreach test_name, extra_args : python_tests depends: depends, args: ['-B', files(test_name)], env: test_env, + timeout: extra_args.get('timeout'), suite: suite, ) From 3d313147f0c3ec8c30a28447c072f6c13520f14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 04:53:23 +0200 Subject: [PATCH 06/11] gio/test-codegen: use valid interface name for FdPassing test So that it can be ran for real, not only tested for generation --- gio/tests/gdbus-test-codegen.c | 22 +++++++++++----------- gio/tests/test-codegen.xml | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/gio/tests/gdbus-test-codegen.c b/gio/tests/gdbus-test-codegen.c index 3fa83e761..9037dd021 100644 --- a/gio/tests/gdbus-test-codegen.c +++ b/gio/tests/gdbus-test-codegen.c @@ -2650,53 +2650,53 @@ test_standalone_interface_info (void) /* ---------------------------------------------------------------------------------------------------- */ static gboolean -handle_hello_fd (FooiGenFDPassing *object, +handle_hello_fd (FooiGenTestFDPassing *object, GDBusMethodInvocation *invocation, GUnixFDList *fd_list, const gchar *arg_greeting) { - foo_igen_fdpassing_complete_hello_fd (object, invocation, fd_list, arg_greeting); + foo_igen_test_fdpassing_complete_hello_fd (object, invocation, fd_list, arg_greeting); return G_DBUS_METHOD_INVOCATION_HANDLED; } #if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64 static gboolean -handle_no_annotation (FooiGenFDPassing *object, +handle_no_annotation (FooiGenTestFDPassing *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); + foo_igen_test_fdpassing_complete_no_annotation (object, invocation, fd_list, arg_greeting, arg_greeting_locale); return G_DBUS_METHOD_INVOCATION_HANDLED; } static gboolean -handle_no_annotation_nested (FooiGenFDPassing *object, +handle_no_annotation_nested (FooiGenTestFDPassing *object, GDBusMethodInvocation *invocation, GUnixFDList *fd_list, GVariant *arg_files) { - foo_igen_fdpassing_complete_no_annotation_nested (object, invocation, fd_list); + foo_igen_test_fdpassing_complete_no_annotation_nested (object, invocation, fd_list); return G_DBUS_METHOD_INVOCATION_HANDLED; } #else static gboolean -handle_no_annotation (FooiGenFDPassing *object, +handle_no_annotation (FooiGenTestFDPassing *object, GDBusMethodInvocation *invocation, GVariant *arg_greeting, const gchar *arg_greeting_locale) { - foo_igen_fdpassing_complete_no_annotation (object, invocation, arg_greeting, arg_greeting_locale); + foo_igen_test_fdpassing_complete_no_annotation (object, invocation, arg_greeting, arg_greeting_locale); return G_DBUS_METHOD_INVOCATION_HANDLED; } static gboolean -handle_no_annotation_nested (FooiGenFDPassing *object, +handle_no_annotation_nested (FooiGenTestFDPassing *object, GDBusMethodInvocation *invocation, GVariant *arg_files) { - foo_igen_fdpassing_complete_no_annotation_nested (object, invocation); + foo_igen_test_fdpassing_complete_no_annotation_nested (object, invocation); return G_DBUS_METHOD_INVOCATION_HANDLED; } #endif @@ -2709,7 +2709,7 @@ handle_no_annotation_nested (FooiGenFDPassing *object, static void test_unix_fd_list (void) { - FooiGenFDPassingIface iface; + FooiGenTestFDPassingIface iface; g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1726"); diff --git a/gio/tests/test-codegen.xml b/gio/tests/test-codegen.xml index 3090cad4a..ae4eaa1ca 100644 --- a/gio/tests/test-codegen.xml +++ b/gio/tests/test-codegen.xml @@ -475,7 +475,7 @@ - + From 6de362fcae99edfc9d86c378384e934b64045e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 05:42:58 +0200 Subject: [PATCH 07/11] gdbus-test-codegen: Add test to check that fd lists can be passed Add a simple implementation to check that fd-passing works when using generated code, this is going to be useful to test special marshalling case. --- gio/tests/gdbus-test-codegen.c | 80 ++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/gio/tests/gdbus-test-codegen.c b/gio/tests/gdbus-test-codegen.c index 9037dd021..31de7f14e 100644 --- a/gio/tests/gdbus-test-codegen.c +++ b/gio/tests/gdbus-test-codegen.c @@ -26,6 +26,7 @@ #include #include "gdbus-tests.h" +#include "gstdio.h" #if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64 #include "gdbus-test-codegen-generated-min-required-2-64.h" @@ -410,6 +411,22 @@ on_handle_check_not_authorized_from_object (FooiGenAuthorize *object, return G_DBUS_METHOD_INVOCATION_HANDLED; } +static gboolean +on_handle_fdpassing_hello_fd (FooiGenMethodThreads *object, + GDBusMethodInvocation *invocation, + GUnixFDList *fd_list, + const gchar *greeting, + gpointer user_data) +{ + g_assert_true (G_IS_UNIX_FD_LIST (fd_list)); + g_assert_cmpuint (g_unix_fd_list_get_length (fd_list), ==, 2); + g_assert_cmpstr (greeting, ==, "Hey fd!"); + foo_igen_test_fdpassing_complete_hello_fd (FOO_IGEN_TEST_FDPASSING (object), + invocation, fd_list, + "I love to receive fds!"); + return G_DBUS_METHOD_INVOCATION_HANDLED; +} + /* ---------------------------------------------------------------------------------------------------- */ static gboolean @@ -430,6 +447,7 @@ static GThread *method_handler_thread = NULL; static FooiGenBar *exported_bar_object = NULL; static FooiGenBat *exported_bat_object = NULL; +static FooiGenTestFDPassing *exported_fd_passing_object = NULL; static FooiGenAuthorize *exported_authorize_object = NULL; static GDBusObjectSkeleton *authorize_enclosing_object = NULL; static FooiGenMethodThreads *exported_thread_object_1 = NULL; @@ -443,6 +461,7 @@ unexport_objects (void) g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (exported_authorize_object)); g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (exported_thread_object_1)); g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (exported_thread_object_2)); + g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (exported_fd_passing_object)); } static void @@ -584,6 +603,17 @@ on_bus_acquired (GDBusConnection *connection, g_assert_cmpint (g_dbus_interface_skeleton_get_flags (G_DBUS_INTERFACE_SKELETON (exported_thread_object_2)), ==, G_DBUS_INTERFACE_SKELETON_FLAGS_NONE); + exported_fd_passing_object = foo_igen_test_fdpassing_skeleton_new (); + g_dbus_interface_skeleton_export (G_DBUS_INTERFACE_SKELETON (exported_fd_passing_object), + connection, + "/fdpassing", + &error); + g_assert_no_error (error); + g_signal_connect (exported_fd_passing_object, + "handle-hello-fd", + G_CALLBACK (on_handle_fdpassing_hello_fd), + NULL); + method_handler_thread = g_thread_self (); } @@ -1125,6 +1155,45 @@ check_bar_proxy (FooiGenBar *proxy, g_variant_unref (array_of_signatures); } +static void +check_fdpassing_proxy (FooiGenTestFDPassing *proxy) +{ + GError *error = NULL; + GUnixFDList *fd_list = g_unix_fd_list_new (); + GUnixFDList *ret_fd_list = NULL; + char *response = NULL; + int fd; + + fd = dup (0); + g_assert_cmpint (g_unix_fd_list_append (fd_list, fd, &error), ==, 0); + g_assert_no_error (error); + g_close (fd, &error); + g_assert_no_error (error); + + fd = dup (0); + g_assert_cmpint (g_unix_fd_list_append (fd_list, fd, &error), ==, 1); + g_assert_no_error (error); + g_close (fd, &error); + g_assert_no_error (error); + + foo_igen_test_fdpassing_call_hello_fd_sync (proxy, "Hey fd!", +#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64 + G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, +#endif + fd_list, + &response, &ret_fd_list, NULL, + &error); + g_assert_no_error (error); + + g_assert_true (G_IS_UNIX_FD_LIST (ret_fd_list)); + g_assert_cmpuint (g_unix_fd_list_get_length (fd_list), ==, 2); + + g_assert_cmpstr (response, ==, "I love to receive fds!"); + g_clear_pointer (&response, g_free); + g_clear_object (&fd_list); + g_clear_object (&ret_fd_list); +} + /* ---------------------------------------------------------------------------------------------------- */ static void @@ -1309,6 +1378,7 @@ check_proxies_in_thread (gpointer user_data) GError *error; FooiGenBar *bar_proxy; FooiGenBat *bat_proxy; + FooiGenTestFDPassing *fd_passing_proxy; FooiGenAuthorize *authorize_proxy; FooiGenMethodThreads *thread_proxy_1; FooiGenMethodThreads *thread_proxy_2; @@ -1370,6 +1440,16 @@ check_proxies_in_thread (gpointer user_data) g_object_unref (thread_proxy_1); g_object_unref (thread_proxy_2); + fd_passing_proxy = foo_igen_test_fdpassing_proxy_new_for_bus_sync (G_BUS_TYPE_SESSION, + G_DBUS_PROXY_FLAGS_NONE, + "org.gtk.GDBus.BindingsTool.Test", + "/fdpassing", + NULL, /* GCancellable* */ + &error); + g_assert_no_error (error); + check_fdpassing_proxy (fd_passing_proxy); + g_object_unref (fd_passing_proxy); + /* Wait for the proxy signals to all be unsubscribed. */ while (g_main_context_iteration (thread_context, FALSE)) { From 8943ceae6cd482d0445027a013935c02f92b4c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 06:19:17 +0200 Subject: [PATCH 08/11] gdbus-codegen: Generate marshallers for method handlers signals Get rid completely of the usage of the generic marshaller in gdbus generated code, using instead specific marshallers. Code is not yet optimized fully since we may still have duplicated functions to be generated. Closes: https://gitlab.gnome.org/GNOME/glib/-/issues/3028 --- gio/gdbus-2.0/codegen/codegen.py | 55 +++++-- gio/gdbus-2.0/codegen/dbustypes.py | 34 ++++ gio/tests/codegen.py | 241 +++++++++++++++++++++++++++++ 3 files changed, 317 insertions(+), 13 deletions(-) diff --git a/gio/gdbus-2.0/codegen/codegen.py b/gio/gdbus-2.0/codegen/codegen.py index a5fd98772..88e08196b 100644 --- a/gio/gdbus-2.0/codegen/codegen.py +++ b/gio/gdbus-2.0/codegen/codegen.py @@ -2136,7 +2136,7 @@ class CodeGenerator: " G_STRUCT_OFFSET (%sIface, handle_%s),\n" " g_signal_accumulator_true_handled,\n" " NULL,\n" # accu_data - " g_cclosure_marshal_generic,\n" + f" {i.name_lower}_method_marshal_{m.name_lower},\n" " G_TYPE_BOOLEAN,\n" " %d,\n" " G_TYPE_DBUS_METHOD_INVOCATION" @@ -2525,34 +2525,44 @@ class CodeGenerator: # --------------------------------------------------------------------------------------------------- - def generate_marshaller(self, func_name, in_args): - self.generate_marshaller_declaration(func_name) + def generate_marshaller(self, func_name, in_args, ret_arg=None): + self.generate_marshaller_declaration(func_name, uses_ret=ret_arg is not None) self.outfile.write("{\n") - self.generate_marshaller_body(func_name, in_args) + self.generate_marshaller_body(func_name, in_args, ret_arg) self.outfile.write("}\n" "\n") - def generate_marshaller_declaration(self, func_name): + def generate_marshaller_declaration(self, func_name, uses_ret=False): self.outfile.write( "static void\n" f"{func_name} (\n" " GClosure *closure,\n" - " GValue *return_value G_GNUC_UNUSED,\n" + f" GValue *return_value{' G_GNUC_UNUSED' if not uses_ret else ''},\n" " unsigned int n_param_values,\n" " const GValue *param_values,\n" " void *invocation_hint G_GNUC_UNUSED,\n" " void *marshal_data)\n" ) - def generate_marshaller_body(self, func_name, in_args=[]): + def generate_marshaller_body(self, func_name, in_args=[], ret_arg=None): marshal_func_type = f"{utils.uscore_to_camel_case(func_name)}Func" self.outfile.write( - f" typedef void (*{marshal_func_type})\n" + f" typedef {ret_arg.ctype_in if ret_arg else 'void '}(*{marshal_func_type})\n" " (void *data1,\n" + "".join([f" {a.ctype_in}arg_{a.name},\n" for a in in_args]) + " void *data2);\n" f" {marshal_func_type} callback;\n" " GCClosure *cc = (GCClosure*) closure;\n" f" void *data1, *data2;\n" + ) + + if ret_arg: + self.outfile.write( + f" {ret_arg.ctype_in}v_return;\n" + "\n" + " g_return_if_fail (return_value != NULL);" + ) + + self.outfile.write( "\n" f" g_return_if_fail (n_param_values == {len(in_args) + 1});\n" "\n" @@ -2570,30 +2580,48 @@ class CodeGenerator: f" callback = ({marshal_func_type})\n" " (marshal_data ? marshal_data : cc->callback);\n" "\n" - " callback (data1,\n" + ) + + prefix = "" + if ret_arg: + self.outfile.write(" v_return =\n") + prefix = 2 * " " + + self.outfile.write( + f"{prefix} callback (data1,\n" + "".join( [ - f" {in_args[i].gvalue_get} (param_values + {i+1}),\n" + f"{prefix} {in_args[i].gvalue_get} (param_values + {i+1}),\n" for i in range(len(in_args)) ] ) - + " data2);\n" + + f"{prefix} data2);\n" ) + if ret_arg: + self.outfile.write( + "\n" f" {ret_arg.gvalue_set} (return_value, v_return);\n" + ) + def generate_marshaller_for_type(self, i, t): - assert isinstance(t, dbustypes.Signal) + assert isinstance(t, (dbustypes.Signal, dbustypes.Method)) kind_uscore = utils.camel_case_to_uscore(t.__class__.__name__.lower()) self.generate_marshaller( func_name=f"{i.name_lower}_{kind_uscore}_marshal_{t.name_lower}", - in_args=t.args, + in_args=t.marshaller_in_args, + ret_arg=t.marshaller_ret_arg, ) def generate_signal_marshallers(self, i): for s in i.signals: self.generate_marshaller_for_type(i, s) + def generate_method_marshallers(self, i): + for m in i.methods: + self.generate_marshaller_for_type(i, m) + # --------------------------------------------------------------------------------------------------- def generate_method_calls(self, i): @@ -5321,6 +5349,7 @@ class CodeGenerator: self.generate_signals_enum_for_interface(i) self.generate_introspection_for_interface(i) self.generate_signal_marshallers(i) + self.generate_method_marshallers(i) self.generate_interface(i) self.generate_property_accessors(i) self.generate_signal_emitters(i) diff --git a/gio/gdbus-2.0/codegen/dbustypes.py b/gio/gdbus-2.0/codegen/dbustypes.py index d5b671e0a..dcc0be9db 100644 --- a/gio/gdbus-2.0/codegen/dbustypes.py +++ b/gio/gdbus-2.0/codegen/dbustypes.py @@ -84,6 +84,7 @@ class Arg: self.format_out = "@" + self.signature self.gvariant_get = "XXX" self.gvalue_get = "g_value_get_variant" + self.gvalue_set = "g_value_take_variant" self.array_annotation = "" if not utils.lookup_annotation( @@ -100,6 +101,7 @@ class Arg: self.format_out = "b" self.gvariant_get = "g_variant_get_boolean" self.gvalue_get = "g_value_get_boolean" + self.gvalue_set = "g_value_set_boolean" elif self.signature == "y": self.ctype_in_g = "guchar " self.ctype_in = "guchar " @@ -111,6 +113,7 @@ class Arg: self.format_out = "y" self.gvariant_get = "g_variant_get_byte" self.gvalue_get = "g_value_get_uchar" + self.gvalue_set = "g_value_set_uchar" elif self.signature == "n": self.ctype_in_g = "gint " self.ctype_in = "gint16 " @@ -122,6 +125,7 @@ class Arg: self.format_out = "n" self.gvariant_get = "g_variant_get_int16" self.gvalue_get = "g_value_get_int" + self.gvalue_set = "g_value_set_int" elif self.signature == "q": self.ctype_in_g = "guint " self.ctype_in = "guint16 " @@ -133,6 +137,7 @@ class Arg: self.format_out = "q" self.gvariant_get = "g_variant_get_uint16" self.gvalue_get = "g_value_get_uint" + self.gvalue_set = "g_value_set_uint" elif self.signature == "i": self.ctype_in_g = "gint " self.ctype_in = "gint " @@ -144,6 +149,7 @@ class Arg: self.format_out = "i" self.gvariant_get = "g_variant_get_int32" self.gvalue_get = "g_value_get_int" + self.gvalue_set = "g_value_set_int" elif self.signature == "u": self.ctype_in_g = "guint " self.ctype_in = "guint " @@ -155,6 +161,7 @@ class Arg: self.format_out = "u" self.gvariant_get = "g_variant_get_uint32" self.gvalue_get = "g_value_get_uint" + self.gvalue_set = "g_value_set_uint" elif self.signature == "x": self.ctype_in_g = "gint64 " self.ctype_in = "gint64 " @@ -166,6 +173,7 @@ class Arg: self.format_out = "x" self.gvariant_get = "g_variant_get_int64" self.gvalue_get = "g_value_get_int64" + self.gvalue_set = "g_value_set_int64" elif self.signature == "t": self.ctype_in_g = "guint64 " self.ctype_in = "guint64 " @@ -177,6 +185,7 @@ class Arg: self.format_out = "t" self.gvariant_get = "g_variant_get_uint64" self.gvalue_get = "g_value_get_uint64" + self.gvalue_set = "g_value_set_uint64" elif self.signature == "d": self.ctype_in_g = "gdouble " self.ctype_in = "gdouble " @@ -188,6 +197,7 @@ class Arg: self.format_out = "d" self.gvariant_get = "g_variant_get_double" self.gvalue_get = "g_value_get_double" + self.gvalue_set = "g_value_set_double" elif self.signature == "s": self.ctype_in_g = "const gchar *" self.ctype_in = "const gchar *" @@ -200,6 +210,7 @@ class Arg: self.format_out = "s" self.gvariant_get = "g_variant_get_string" self.gvalue_get = "g_value_get_string" + self.gvalue_set = "g_value_set_string" elif self.signature == "o": self.ctype_in_g = "const gchar *" self.ctype_in = "const gchar *" @@ -212,6 +223,7 @@ class Arg: self.format_out = "o" self.gvariant_get = "g_variant_get_string" self.gvalue_get = "g_value_get_string" + self.gvalue_set = "g_value_set_string" elif self.signature == "g": self.ctype_in_g = "const gchar *" self.ctype_in = "const gchar *" @@ -224,6 +236,7 @@ class Arg: self.format_out = "g" self.gvariant_get = "g_variant_get_string" self.gvalue_get = "g_value_get_string" + self.gvalue_set = "g_value_set_string" elif self.signature == "ay": self.ctype_in_g = "const gchar *" self.ctype_in = "const gchar *" @@ -236,6 +249,7 @@ class Arg: self.format_out = "^ay" self.gvariant_get = "g_variant_get_bytestring" self.gvalue_get = "g_value_get_string" + self.gvalue_set = "g_value_set_string" elif self.signature == "as": self.ctype_in_g = "const gchar *const *" self.ctype_in = "const gchar *const *" @@ -248,6 +262,7 @@ class Arg: self.format_out = "^as" self.gvariant_get = "g_variant_get_strv" self.gvalue_get = "g_value_get_boxed" + self.gvalue_set = "g_value_take_boxed" self.array_annotation = "(array zero-terminated=1)" elif self.signature == "ao": self.ctype_in_g = "const gchar *const *" @@ -261,6 +276,7 @@ class Arg: self.format_out = "^ao" self.gvariant_get = "g_variant_get_objv" self.gvalue_get = "g_value_get_boxed" + self.gvalue_set = "g_value_take_boxed" self.array_annotation = "(array zero-terminated=1)" elif self.signature == "aay": self.ctype_in_g = "const gchar *const *" @@ -274,6 +290,7 @@ class Arg: self.format_out = "^aay" self.gvariant_get = "g_variant_get_bytestring_array" self.gvalue_get = "g_value_get_boxed" + self.gvalue_set = "g_value_take_boxed" self.array_annotation = "(array zero-terminated=1)" for a in self.annotations: @@ -336,6 +353,20 @@ class Method: if utils.lookup_annotation(self.annotations, "org.gtk.GDBus.C.UnixFD"): self.unix_fd = True + self.marshaller_ret_arg = Arg("return", "b") + self.marshaller_ret_arg.post_process(None, None, None, None, None) + + method_invocation_arg = Arg("method_invocation", None) + method_invocation_arg.ctype_in = "GDBusMethodInvocation *" + method_invocation_arg.gvalue_get = "g_value_get_object" + self.marshaller_in_args = [method_invocation_arg] + self.in_args + + if self.unix_fd: + fd_list_arg = Arg("fd_list", None) + fd_list_arg.ctype_in = "GUnixFDList *" + fd_list_arg.gvalue_get = "g_value_get_object" + self.marshaller_in_args.insert(0, fd_list_arg) + for a in self.annotations: a.post_process(interface_prefix, cns, cns_upper, cns_lower, self) @@ -386,6 +417,9 @@ class Signal: ): self.deprecated = True + self.marshaller_ret_arg = None + self.marshaller_in_args = self.args + for a in self.annotations: a.post_process(interface_prefix, cns, cns_upper, cns_lower, self) diff --git a/gio/tests/codegen.py b/gio/tests/codegen.py index d591df192..746517204 100644 --- a/gio/tests/codegen.py +++ b/gio/tests/codegen.py @@ -903,6 +903,247 @@ G_END_DECLS ) index += 1 + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_methods_marshaller_simple_method(self): + """Test that methods marshaller is generated for simple method""" + interface_xml = """ + + + + + + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = "org_project_callable_iface_method_marshal_simple_method" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + func_name = "org_project_other_callable_iface_method_marshal_simple_method" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + self.assertIs(stripped_out.count("g_value_get_object (param_values + 1)"), 2) + self.assertIs( + stripped_out.count("g_value_set_boolean (return_value, v_return);"), 2 + ) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_methods_marshaller_single_typed_in_args(self): + """Test that methods marshallers are generated for each known type""" + for t, props in self.ARGUMENTS_TYPES.items(): + camel_type = t[0].upper() + t[1:] + interface_xml = f""" + + + + + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertEqual(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = ( + f"org_project_useful_interface_method_marshal_single_arg_method_{t}" + ) + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + self.assertIs( + stripped_out.count("g_value_get_object (param_values + 1)"), 1 + ) + self.assertIs( + stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 + ) + self.assertIs( + stripped_out.count( + f"g_value_get_{props['value_type']} (param_values + 2)" + ), + 1, + ) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_methods_marshaller_single_typed_out_args(self): + """Test that methods marshallers are generated for each known type""" + for t, props in self.ARGUMENTS_TYPES.items(): + camel_type = t[0].upper() + t[1:] + interface_xml = f""" + + + + + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertEqual(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = ( + f"org_project_useful_interface_method_marshal_single_arg_method_{t}" + ) + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + self.assertIs( + stripped_out.count("g_value_get_object (param_values + 1)"), 1 + ) + self.assertIs( + stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 + ) + self.assertIs(stripped_out.count("(param_values + 2)"), 0) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_methods_marshallers_multiple_in_args(self): + """Test that methods marshallers are generated""" + generated_args = [ + f"\n" + for t, props in self.ARGUMENTS_TYPES.items() + ] + + interface_xml = f""" + + + + {''.join(generated_args)} + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = f"org_project_callable_iface_method_marshal_method_with_many_args" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + # Check access to MultipleArgsMethod arguments + index = 1 + self.assertIs( + stripped_out.count(f"g_value_get_object (param_values + {index})"), 1 + ) + index += 1 + + for props in self.ARGUMENTS_TYPES.values(): + self.assertIs( + stripped_out.count( + f"g_value_get_{props['value_type']} (param_values + {index})" + ), + 1, + ) + index += 1 + + self.assertIs( + stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 + ) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_methods_marshallers_multiple_out_args(self): + """Test that methods marshallers are generated""" + generated_args = [ + f"\n" + for t, props in self.ARGUMENTS_TYPES.items() + ] + + interface_xml = f""" + + + + {''.join(generated_args)} + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = f"org_project_callable_iface_method_marshal_method_with_many_args" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + # Check access to MultipleArgsMethod arguments + index = 1 + self.assertIs( + stripped_out.count(f"g_value_get_object (param_values + {index})"), 1 + ) + index += 1 + + for index in range(index, len(self.ARGUMENTS_TYPES)): + self.assertIs(stripped_out.count(f"(param_values + {index})"), 0) + + self.assertIs( + stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 + ) + + @unittest.skipIf(on_win32(), "requires /dev/stdout") + def test_generate_methods_marshallers_with_unix_fds(self): + """Test an interface with `h` arguments""" + interface_xml = """ + + + + + + + + + """ + + result = self.runCodegenWithInterface( + interface_xml, "--output", "/dev/stdout", "--body" + ) + stripped_out = result.out.strip() + self.assertFalse(result.err) + self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0) + + func_name = f"test_fdpassing_method_marshal_hello_fd" + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) + + index = 1 + self.assertIs( + stripped_out.count(f"g_value_get_object (param_values + {index})"), 1 + ) + index += 1 + + self.assertIs( + stripped_out.count(f"g_value_get_object (param_values + {index})"), 1 + ) + + index += 1 + self.assertIs( + stripped_out.count(f"g_value_get_string (param_values + {index})"), 1 + ) + index += 1 + + self.assertIs( + stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 + ) + def test_generate_valid_docbook(self): """Test the basic functionality of the docbook generator.""" xml_contents = """ From c5d900423600a52a1a88d8e2e40d6c0c7340643c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 19:24:03 +0200 Subject: [PATCH 09/11] gdbus-codegen: Use direct-access GValue macros for marshallers and GValues This is the same we're doing in code generated by glib-genmarshaller and what gmarshal does internally. Since the generated gdbus C code can be considered private too, this is safe to do, and will allow faster access to GValue objects. --- gio/gdbus-2.0/codegen/codegen.py | 49 +++++++++++++++++++ gio/gdbus-2.0/codegen/dbustypes.py | 38 +++++++-------- gio/tests/codegen.py | 78 +++++++++++++++++++++++++----- 3 files changed, 134 insertions(+), 31 deletions(-) diff --git a/gio/gdbus-2.0/codegen/codegen.py b/gio/gdbus-2.0/codegen/codegen.py index 88e08196b..f96a6f5ce 100644 --- a/gio/gdbus-2.0/codegen/codegen.py +++ b/gio/gdbus-2.0/codegen/codegen.py @@ -1486,6 +1486,55 @@ class CodeGenerator: "#ifdef G_OS_UNIX\n" "# include \n" "#endif\n" "\n" ) + self.outfile.write( + """#ifdef G_ENABLE_DEBUG +#define g_marshal_value_peek_boolean(v) g_value_get_boolean (v) +#define g_marshal_value_peek_char(v) g_value_get_schar (v) +#define g_marshal_value_peek_uchar(v) g_value_get_uchar (v) +#define g_marshal_value_peek_int(v) g_value_get_int (v) +#define g_marshal_value_peek_uint(v) g_value_get_uint (v) +#define g_marshal_value_peek_long(v) g_value_get_long (v) +#define g_marshal_value_peek_ulong(v) g_value_get_ulong (v) +#define g_marshal_value_peek_int64(v) g_value_get_int64 (v) +#define g_marshal_value_peek_uint64(v) g_value_get_uint64 (v) +#define g_marshal_value_peek_enum(v) g_value_get_enum (v) +#define g_marshal_value_peek_flags(v) g_value_get_flags (v) +#define g_marshal_value_peek_float(v) g_value_get_float (v) +#define g_marshal_value_peek_double(v) g_value_get_double (v) +#define g_marshal_value_peek_string(v) (char*) g_value_get_string (v) +#define g_marshal_value_peek_param(v) g_value_get_param (v) +#define g_marshal_value_peek_boxed(v) g_value_get_boxed (v) +#define g_marshal_value_peek_pointer(v) g_value_get_pointer (v) +#define g_marshal_value_peek_object(v) g_value_get_object (v) +#define g_marshal_value_peek_variant(v) g_value_get_variant (v) +#else /* !G_ENABLE_DEBUG */ +/* WARNING: This code accesses GValues directly, which is UNSUPPORTED API. + * Do not access GValues directly in your code. Instead, use the + * g_value_get_*() functions + */ +#define g_marshal_value_peek_boolean(v) (v)->data[0].v_int +#define g_marshal_value_peek_char(v) (v)->data[0].v_int +#define g_marshal_value_peek_uchar(v) (v)->data[0].v_uint +#define g_marshal_value_peek_int(v) (v)->data[0].v_int +#define g_marshal_value_peek_uint(v) (v)->data[0].v_uint +#define g_marshal_value_peek_long(v) (v)->data[0].v_long +#define g_marshal_value_peek_ulong(v) (v)->data[0].v_ulong +#define g_marshal_value_peek_int64(v) (v)->data[0].v_int64 +#define g_marshal_value_peek_uint64(v) (v)->data[0].v_uint64 +#define g_marshal_value_peek_enum(v) (v)->data[0].v_long +#define g_marshal_value_peek_flags(v) (v)->data[0].v_ulong +#define g_marshal_value_peek_float(v) (v)->data[0].v_float +#define g_marshal_value_peek_double(v) (v)->data[0].v_double +#define g_marshal_value_peek_string(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_param(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_boxed(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_pointer(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_object(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_variant(v) (v)->data[0].v_pointer +#endif /* !G_ENABLE_DEBUG */""" + "\n\n" + ) + self.outfile.write( "typedef struct\n" "{\n" diff --git a/gio/gdbus-2.0/codegen/dbustypes.py b/gio/gdbus-2.0/codegen/dbustypes.py index dcc0be9db..42ca7dae8 100644 --- a/gio/gdbus-2.0/codegen/dbustypes.py +++ b/gio/gdbus-2.0/codegen/dbustypes.py @@ -83,7 +83,7 @@ class Arg: self.format_in = "@" + self.signature self.format_out = "@" + self.signature self.gvariant_get = "XXX" - self.gvalue_get = "g_value_get_variant" + self.gvalue_get = "g_marshal_value_peek_variant" self.gvalue_set = "g_value_take_variant" self.array_annotation = "" @@ -100,7 +100,7 @@ class Arg: self.format_in = "b" self.format_out = "b" self.gvariant_get = "g_variant_get_boolean" - self.gvalue_get = "g_value_get_boolean" + self.gvalue_get = "g_marshal_value_peek_boolean" self.gvalue_set = "g_value_set_boolean" elif self.signature == "y": self.ctype_in_g = "guchar " @@ -112,7 +112,7 @@ class Arg: self.format_in = "y" self.format_out = "y" self.gvariant_get = "g_variant_get_byte" - self.gvalue_get = "g_value_get_uchar" + self.gvalue_get = "g_marshal_value_peek_uchar" self.gvalue_set = "g_value_set_uchar" elif self.signature == "n": self.ctype_in_g = "gint " @@ -124,7 +124,7 @@ class Arg: self.format_in = "n" self.format_out = "n" self.gvariant_get = "g_variant_get_int16" - self.gvalue_get = "g_value_get_int" + self.gvalue_get = "g_marshal_value_peek_int" self.gvalue_set = "g_value_set_int" elif self.signature == "q": self.ctype_in_g = "guint " @@ -136,7 +136,7 @@ class Arg: self.format_in = "q" self.format_out = "q" self.gvariant_get = "g_variant_get_uint16" - self.gvalue_get = "g_value_get_uint" + self.gvalue_get = "g_marshal_value_peek_uint" self.gvalue_set = "g_value_set_uint" elif self.signature == "i": self.ctype_in_g = "gint " @@ -148,7 +148,7 @@ class Arg: self.format_in = "i" self.format_out = "i" self.gvariant_get = "g_variant_get_int32" - self.gvalue_get = "g_value_get_int" + self.gvalue_get = "g_marshal_value_peek_int" self.gvalue_set = "g_value_set_int" elif self.signature == "u": self.ctype_in_g = "guint " @@ -160,7 +160,7 @@ class Arg: self.format_in = "u" self.format_out = "u" self.gvariant_get = "g_variant_get_uint32" - self.gvalue_get = "g_value_get_uint" + self.gvalue_get = "g_marshal_value_peek_uint" self.gvalue_set = "g_value_set_uint" elif self.signature == "x": self.ctype_in_g = "gint64 " @@ -172,7 +172,7 @@ class Arg: self.format_in = "x" self.format_out = "x" self.gvariant_get = "g_variant_get_int64" - self.gvalue_get = "g_value_get_int64" + self.gvalue_get = "g_marshal_value_peek_int64" self.gvalue_set = "g_value_set_int64" elif self.signature == "t": self.ctype_in_g = "guint64 " @@ -184,7 +184,7 @@ class Arg: self.format_in = "t" self.format_out = "t" self.gvariant_get = "g_variant_get_uint64" - self.gvalue_get = "g_value_get_uint64" + self.gvalue_get = "g_marshal_value_peek_uint64" self.gvalue_set = "g_value_set_uint64" elif self.signature == "d": self.ctype_in_g = "gdouble " @@ -196,7 +196,7 @@ class Arg: self.format_in = "d" self.format_out = "d" self.gvariant_get = "g_variant_get_double" - self.gvalue_get = "g_value_get_double" + self.gvalue_get = "g_marshal_value_peek_double" self.gvalue_set = "g_value_set_double" elif self.signature == "s": self.ctype_in_g = "const gchar *" @@ -209,7 +209,7 @@ class Arg: self.format_in = "s" self.format_out = "s" self.gvariant_get = "g_variant_get_string" - self.gvalue_get = "g_value_get_string" + self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" elif self.signature == "o": self.ctype_in_g = "const gchar *" @@ -222,7 +222,7 @@ class Arg: self.format_in = "o" self.format_out = "o" self.gvariant_get = "g_variant_get_string" - self.gvalue_get = "g_value_get_string" + self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" elif self.signature == "g": self.ctype_in_g = "const gchar *" @@ -235,7 +235,7 @@ class Arg: self.format_in = "g" self.format_out = "g" self.gvariant_get = "g_variant_get_string" - self.gvalue_get = "g_value_get_string" + self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" elif self.signature == "ay": self.ctype_in_g = "const gchar *" @@ -248,7 +248,7 @@ class Arg: self.format_in = "^ay" self.format_out = "^ay" self.gvariant_get = "g_variant_get_bytestring" - self.gvalue_get = "g_value_get_string" + self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" elif self.signature == "as": self.ctype_in_g = "const gchar *const *" @@ -261,7 +261,7 @@ class Arg: self.format_in = "^as" self.format_out = "^as" self.gvariant_get = "g_variant_get_strv" - self.gvalue_get = "g_value_get_boxed" + self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" self.array_annotation = "(array zero-terminated=1)" elif self.signature == "ao": @@ -275,7 +275,7 @@ class Arg: self.format_in = "^ao" self.format_out = "^ao" self.gvariant_get = "g_variant_get_objv" - self.gvalue_get = "g_value_get_boxed" + self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" self.array_annotation = "(array zero-terminated=1)" elif self.signature == "aay": @@ -289,7 +289,7 @@ class Arg: self.format_in = "^aay" self.format_out = "^aay" self.gvariant_get = "g_variant_get_bytestring_array" - self.gvalue_get = "g_value_get_boxed" + self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" self.array_annotation = "(array zero-terminated=1)" @@ -358,13 +358,13 @@ class Method: method_invocation_arg = Arg("method_invocation", None) method_invocation_arg.ctype_in = "GDBusMethodInvocation *" - method_invocation_arg.gvalue_get = "g_value_get_object" + method_invocation_arg.gvalue_get = "g_marshal_value_peek_object" self.marshaller_in_args = [method_invocation_arg] + self.in_args if self.unix_fd: fd_list_arg = Arg("fd_list", None) fd_list_arg.ctype_in = "GUnixFDList *" - fd_list_arg.gvalue_get = "g_value_get_object" + fd_list_arg.gvalue_get = "g_marshal_value_peek_object" self.marshaller_in_args.insert(0, fd_list_arg) for a in self.annotations: diff --git a/gio/tests/codegen.py b/gio/tests/codegen.py index 746517204..bd2543118 100644 --- a/gio/tests/codegen.py +++ b/gio/tests/codegen.py @@ -147,6 +147,51 @@ class TestCodegen(unittest.TestCase): "#ifdef G_OS_UNIX\n" "# include \n" "#endif", + "private_gvalues_getters": """#ifdef G_ENABLE_DEBUG +#define g_marshal_value_peek_boolean(v) g_value_get_boolean (v) +#define g_marshal_value_peek_char(v) g_value_get_schar (v) +#define g_marshal_value_peek_uchar(v) g_value_get_uchar (v) +#define g_marshal_value_peek_int(v) g_value_get_int (v) +#define g_marshal_value_peek_uint(v) g_value_get_uint (v) +#define g_marshal_value_peek_long(v) g_value_get_long (v) +#define g_marshal_value_peek_ulong(v) g_value_get_ulong (v) +#define g_marshal_value_peek_int64(v) g_value_get_int64 (v) +#define g_marshal_value_peek_uint64(v) g_value_get_uint64 (v) +#define g_marshal_value_peek_enum(v) g_value_get_enum (v) +#define g_marshal_value_peek_flags(v) g_value_get_flags (v) +#define g_marshal_value_peek_float(v) g_value_get_float (v) +#define g_marshal_value_peek_double(v) g_value_get_double (v) +#define g_marshal_value_peek_string(v) (char*) g_value_get_string (v) +#define g_marshal_value_peek_param(v) g_value_get_param (v) +#define g_marshal_value_peek_boxed(v) g_value_get_boxed (v) +#define g_marshal_value_peek_pointer(v) g_value_get_pointer (v) +#define g_marshal_value_peek_object(v) g_value_get_object (v) +#define g_marshal_value_peek_variant(v) g_value_get_variant (v) +#else /* !G_ENABLE_DEBUG */ +/* WARNING: This code accesses GValues directly, which is UNSUPPORTED API. + * Do not access GValues directly in your code. Instead, use the + * g_value_get_*() functions + */ +#define g_marshal_value_peek_boolean(v) (v)->data[0].v_int +#define g_marshal_value_peek_char(v) (v)->data[0].v_int +#define g_marshal_value_peek_uchar(v) (v)->data[0].v_uint +#define g_marshal_value_peek_int(v) (v)->data[0].v_int +#define g_marshal_value_peek_uint(v) (v)->data[0].v_uint +#define g_marshal_value_peek_long(v) (v)->data[0].v_long +#define g_marshal_value_peek_ulong(v) (v)->data[0].v_ulong +#define g_marshal_value_peek_int64(v) (v)->data[0].v_int64 +#define g_marshal_value_peek_uint64(v) (v)->data[0].v_uint64 +#define g_marshal_value_peek_enum(v) (v)->data[0].v_long +#define g_marshal_value_peek_flags(v) (v)->data[0].v_ulong +#define g_marshal_value_peek_float(v) (v)->data[0].v_float +#define g_marshal_value_peek_double(v) (v)->data[0].v_double +#define g_marshal_value_peek_string(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_param(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_boxed(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_pointer(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_object(v) (v)->data[0].v_pointer +#define g_marshal_value_peek_variant(v) (v)->data[0].v_pointer +#endif /* !G_ENABLE_DEBUG */""", "standard_typedefs_and_helpers": "typedef struct\n" "{\n" " GDBusArgInfo parent_struct;\n" @@ -346,6 +391,8 @@ G_END_DECLS {standard_header_includes} +{private_gvalues_getters} + {standard_typedefs_and_helpers}""".format( **result.subs ), @@ -854,7 +901,7 @@ G_END_DECLS self.assertIs(stripped_out.count(f"{func_name} ("), 1) self.assertIs( stripped_out.count( - f"g_value_get_{props['value_type']} (param_values + 1)" + f"g_marshal_value_peek_{props['value_type']} (param_values + 1)" ), 1, ) @@ -897,7 +944,7 @@ G_END_DECLS for props in self.ARGUMENTS_TYPES.values(): self.assertIs( stripped_out.count( - f"g_value_get_{props['value_type']} (param_values + {index})" + f"g_marshal_value_peek_{props['value_type']} (param_values + {index})" ), 1, ) @@ -931,7 +978,9 @@ G_END_DECLS self.assertIs(stripped_out.count(f"{func_name},"), 1) self.assertIs(stripped_out.count(f"{func_name} ("), 1) - self.assertIs(stripped_out.count("g_value_get_object (param_values + 1)"), 2) + self.assertIs( + stripped_out.count("g_marshal_value_peek_object (param_values + 1)"), 2 + ) self.assertIs( stripped_out.count("g_value_set_boolean (return_value, v_return);"), 2 ) @@ -963,14 +1012,14 @@ G_END_DECLS self.assertIs(stripped_out.count(f"{func_name},"), 1) self.assertIs(stripped_out.count(f"{func_name} ("), 1) self.assertIs( - stripped_out.count("g_value_get_object (param_values + 1)"), 1 + stripped_out.count("g_marshal_value_peek_object (param_values + 1)"), 1 ) self.assertIs( stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 ) self.assertIs( stripped_out.count( - f"g_value_get_{props['value_type']} (param_values + 2)" + f"g_marshal_value_peek_{props['value_type']} (param_values + 2)" ), 1, ) @@ -1002,7 +1051,7 @@ G_END_DECLS self.assertIs(stripped_out.count(f"{func_name},"), 1) self.assertIs(stripped_out.count(f"{func_name} ("), 1) self.assertIs( - stripped_out.count("g_value_get_object (param_values + 1)"), 1 + stripped_out.count("g_marshal_value_peek_object (param_values + 1)"), 1 ) self.assertIs( stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 @@ -1040,14 +1089,15 @@ G_END_DECLS # Check access to MultipleArgsMethod arguments index = 1 self.assertIs( - stripped_out.count(f"g_value_get_object (param_values + {index})"), 1 + stripped_out.count(f"g_marshal_value_peek_object (param_values + {index})"), + 1, ) index += 1 for props in self.ARGUMENTS_TYPES.values(): self.assertIs( stripped_out.count( - f"g_value_get_{props['value_type']} (param_values + {index})" + f"g_marshal_value_peek_{props['value_type']} (param_values + {index})" ), 1, ) @@ -1088,7 +1138,8 @@ G_END_DECLS # Check access to MultipleArgsMethod arguments index = 1 self.assertIs( - stripped_out.count(f"g_value_get_object (param_values + {index})"), 1 + stripped_out.count(f"g_marshal_value_peek_object (param_values + {index})"), + 1, ) index += 1 @@ -1126,17 +1177,20 @@ G_END_DECLS index = 1 self.assertIs( - stripped_out.count(f"g_value_get_object (param_values + {index})"), 1 + stripped_out.count(f"g_marshal_value_peek_object (param_values + {index})"), + 1, ) index += 1 self.assertIs( - stripped_out.count(f"g_value_get_object (param_values + {index})"), 1 + stripped_out.count(f"g_marshal_value_peek_object (param_values + {index})"), + 1, ) index += 1 self.assertIs( - stripped_out.count(f"g_value_get_string (param_values + {index})"), 1 + stripped_out.count(f"g_marshal_value_peek_string (param_values + {index})"), + 1, ) index += 1 From 23180d433eea3b437c34f1ed8e3c35370f6e12e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 20:00:28 +0200 Subject: [PATCH 10/11] gdbus-codegen: Try to reuse default glib marshallers for simpler signals marshallers If a signal signautre is simple enough we can just reuse the default marshallers by simply creating a wrapper marshaller --- gio/gdbus-2.0/codegen/codegen.py | 42 ++++++++++++++++++++++++------ gio/gdbus-2.0/codegen/dbustypes.py | 17 ++++++++++++ gio/tests/codegen.py | 31 ++++++++++++++++------ 3 files changed, 74 insertions(+), 16 deletions(-) diff --git a/gio/gdbus-2.0/codegen/codegen.py b/gio/gdbus-2.0/codegen/codegen.py index f96a6f5ce..7e0a35a01 100644 --- a/gio/gdbus-2.0/codegen/codegen.py +++ b/gio/gdbus-2.0/codegen/codegen.py @@ -2580,15 +2580,32 @@ class CodeGenerator: self.generate_marshaller_body(func_name, in_args, ret_arg) self.outfile.write("}\n" "\n") - def generate_marshaller_declaration(self, func_name, uses_ret=False): + def generate_marshaller_wrapper(self, wrapper_name, wrapped_func): + self.generate_marshaller_declaration( + wrapper_name, + uses_ret=True, + uses_hint=True, + inline=True, + ) + self.outfile.write("{\n") self.outfile.write( - "static void\n" + f" {wrapped_func} (closure,\n" + " return_value, n_param_values, param_values, " + "invocation_hint, marshal_data);\n" + ) + self.outfile.write("}\n" "\n") + + def generate_marshaller_declaration( + self, func_name, uses_ret=False, uses_hint=False, inline=False + ): + self.outfile.write( + f"{'inline ' if inline else ''}static void\n" f"{func_name} (\n" " GClosure *closure,\n" f" GValue *return_value{' G_GNUC_UNUSED' if not uses_ret else ''},\n" " unsigned int n_param_values,\n" " const GValue *param_values,\n" - " void *invocation_hint G_GNUC_UNUSED,\n" + f" void *invocation_hint{' G_GNUC_UNUSED' if not uses_hint else ''},\n" " void *marshal_data)\n" ) @@ -2656,12 +2673,21 @@ class CodeGenerator: assert isinstance(t, (dbustypes.Signal, dbustypes.Method)) kind_uscore = utils.camel_case_to_uscore(t.__class__.__name__.lower()) + func_name = f"{i.name_lower}_{kind_uscore}_marshal_{t.name_lower}" - self.generate_marshaller( - func_name=f"{i.name_lower}_{kind_uscore}_marshal_{t.name_lower}", - in_args=t.marshaller_in_args, - ret_arg=t.marshaller_ret_arg, - ) + if not t.marshaller_ret_arg: + if not t.marshaller_in_args: + self.generate_marshaller_wrapper( + func_name, "g_cclosure_marshal_VOID__VOID" + ) + return + elif len(t.marshaller_in_args) == 1 and t.args[0].gclosure_marshaller: + self.generate_marshaller_wrapper( + func_name, t.args[0].gclosure_marshaller + ) + return + + self.generate_marshaller(func_name, t.marshaller_in_args, t.marshaller_ret_arg) def generate_signal_marshallers(self, i): for s in i.signals: diff --git a/gio/gdbus-2.0/codegen/dbustypes.py b/gio/gdbus-2.0/codegen/dbustypes.py index 42ca7dae8..60ba6c4fa 100644 --- a/gio/gdbus-2.0/codegen/dbustypes.py +++ b/gio/gdbus-2.0/codegen/dbustypes.py @@ -85,6 +85,7 @@ class Arg: self.gvariant_get = "XXX" self.gvalue_get = "g_marshal_value_peek_variant" self.gvalue_set = "g_value_take_variant" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__VARIANT" self.array_annotation = "" if not utils.lookup_annotation( @@ -102,6 +103,7 @@ class Arg: self.gvariant_get = "g_variant_get_boolean" self.gvalue_get = "g_marshal_value_peek_boolean" self.gvalue_set = "g_value_set_boolean" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__BOOLEAN" elif self.signature == "y": self.ctype_in_g = "guchar " self.ctype_in = "guchar " @@ -114,6 +116,7 @@ class Arg: self.gvariant_get = "g_variant_get_byte" self.gvalue_get = "g_marshal_value_peek_uchar" self.gvalue_set = "g_value_set_uchar" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__UCHAR" elif self.signature == "n": self.ctype_in_g = "gint " self.ctype_in = "gint16 " @@ -126,6 +129,7 @@ class Arg: self.gvariant_get = "g_variant_get_int16" self.gvalue_get = "g_marshal_value_peek_int" self.gvalue_set = "g_value_set_int" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__INT" elif self.signature == "q": self.ctype_in_g = "guint " self.ctype_in = "guint16 " @@ -138,6 +142,7 @@ class Arg: self.gvariant_get = "g_variant_get_uint16" self.gvalue_get = "g_marshal_value_peek_uint" self.gvalue_set = "g_value_set_uint" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__UINT" elif self.signature == "i": self.ctype_in_g = "gint " self.ctype_in = "gint " @@ -150,6 +155,7 @@ class Arg: self.gvariant_get = "g_variant_get_int32" self.gvalue_get = "g_marshal_value_peek_int" self.gvalue_set = "g_value_set_int" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__INT" elif self.signature == "u": self.ctype_in_g = "guint " self.ctype_in = "guint " @@ -162,6 +168,7 @@ class Arg: self.gvariant_get = "g_variant_get_uint32" self.gvalue_get = "g_marshal_value_peek_uint" self.gvalue_set = "g_value_set_uint" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__UINT" elif self.signature == "x": self.ctype_in_g = "gint64 " self.ctype_in = "gint64 " @@ -174,6 +181,7 @@ class Arg: self.gvariant_get = "g_variant_get_int64" self.gvalue_get = "g_marshal_value_peek_int64" self.gvalue_set = "g_value_set_int64" + self.gclosure_marshaller = None elif self.signature == "t": self.ctype_in_g = "guint64 " self.ctype_in = "guint64 " @@ -186,6 +194,7 @@ class Arg: self.gvariant_get = "g_variant_get_uint64" self.gvalue_get = "g_marshal_value_peek_uint64" self.gvalue_set = "g_value_set_uint64" + self.gclosure_marshaller = None elif self.signature == "d": self.ctype_in_g = "gdouble " self.ctype_in = "gdouble " @@ -198,6 +207,7 @@ class Arg: self.gvariant_get = "g_variant_get_double" self.gvalue_get = "g_marshal_value_peek_double" self.gvalue_set = "g_value_set_double" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__DOUBLE" elif self.signature == "s": self.ctype_in_g = "const gchar *" self.ctype_in = "const gchar *" @@ -211,6 +221,7 @@ class Arg: self.gvariant_get = "g_variant_get_string" self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__STRING" elif self.signature == "o": self.ctype_in_g = "const gchar *" self.ctype_in = "const gchar *" @@ -224,6 +235,7 @@ class Arg: self.gvariant_get = "g_variant_get_string" self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__STRING" elif self.signature == "g": self.ctype_in_g = "const gchar *" self.ctype_in = "const gchar *" @@ -237,6 +249,7 @@ class Arg: self.gvariant_get = "g_variant_get_string" self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__STRING" elif self.signature == "ay": self.ctype_in_g = "const gchar *" self.ctype_in = "const gchar *" @@ -250,6 +263,7 @@ class Arg: self.gvariant_get = "g_variant_get_bytestring" self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__STRING" elif self.signature == "as": self.ctype_in_g = "const gchar *const *" self.ctype_in = "const gchar *const *" @@ -263,6 +277,7 @@ class Arg: self.gvariant_get = "g_variant_get_strv" self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__BOXED" self.array_annotation = "(array zero-terminated=1)" elif self.signature == "ao": self.ctype_in_g = "const gchar *const *" @@ -277,6 +292,7 @@ class Arg: self.gvariant_get = "g_variant_get_objv" self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__BOXED" self.array_annotation = "(array zero-terminated=1)" elif self.signature == "aay": self.ctype_in_g = "const gchar *const *" @@ -291,6 +307,7 @@ class Arg: self.gvariant_get = "g_variant_get_bytestring_array" self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" + self.gclosure_marshaller = "g_cclosure_marshal_VOID__BOXED" self.array_annotation = "(array zero-terminated=1)" for a in self.annotations: diff --git a/gio/tests/codegen.py b/gio/tests/codegen.py index bd2543118..2cdd59b5a 100644 --- a/gio/tests/codegen.py +++ b/gio/tests/codegen.py @@ -67,8 +67,8 @@ class TestCodegen(unittest.TestCase): "q": {"value_type": "uint"}, "i": {"value_type": "int"}, "u": {"value_type": "uint"}, - "x": {"value_type": "int64"}, - "t": {"value_type": "uint64"}, + "x": {"value_type": "int64", "lacks_marshaller": True}, + "t": {"value_type": "uint64", "lacks_marshaller": True}, "d": {"value_type": "double"}, "s": {"value_type": "string"}, "o": {"value_type": "string"}, @@ -867,10 +867,12 @@ G_END_DECLS func_name = "org_project_signaling_iface_signal_marshal_simple_signal" self.assertIs(stripped_out.count(f"{func_name},"), 1) self.assertIs(stripped_out.count(f"{func_name} ("), 1) + self.assertIs(stripped_out.count("g_cclosure_marshal_VOID__VOID (closure"), 2) func_name = "org_project_other_signaling_iface_signal_marshal_simple_signal" self.assertIs(stripped_out.count(f"{func_name},"), 1) self.assertIs(stripped_out.count(f"{func_name} ("), 1) + self.assertIs(stripped_out.count("g_cclosure_marshal_VOID__VOID (closure"), 2) @unittest.skipIf(on_win32(), "requires /dev/stdout") def test_generate_signals_marshaller_single_typed_args(self): @@ -894,17 +896,30 @@ G_END_DECLS self.assertFalse(result.err) self.assertEqual(stripped_out.count("g_cclosure_marshal_generic"), 0) + self.assertIs( + stripped_out.count("g_cclosure_marshal_VOID__VOID (closure"), 1 + ) + func_name = ( f"org_project_signaling_iface_signal_marshal_single_arg_signal_{t}" ) self.assertIs(stripped_out.count(f"{func_name},"), 1) self.assertIs(stripped_out.count(f"{func_name} ("), 1) - self.assertIs( - stripped_out.count( - f"g_marshal_value_peek_{props['value_type']} (param_values + 1)" - ), - 1, - ) + + if props.get("lacks_marshaller", False): + self.assertIs( + stripped_out.count( + f"g_marshal_value_peek_{props['value_type']} (param_values + 1)" + ), + 1, + ) + else: + self.assertIs( + stripped_out.count( + f"g_cclosure_marshal_VOID__{props['value_type'].upper()} (closure" + ), + 1, + ) @unittest.skipIf(on_win32(), "requires /dev/stdout") def test_generate_signals_marshallers_multiple_args(self): From 27b7559c91c870d80afd6fe727ba62fb7697d970 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 30 Jun 2023 22:35:20 +0200 Subject: [PATCH 11/11] gdbus-codegen: Define only one marshaller per signature reusing across interfaces Avoid generating more code than needed, so other than continuing using the generic glib marshallers when possible, define once the custom ones we need for each file we generate. The marshallers are then re-used across all the interfaces defined without duplicating the code size. --- gio/gdbus-2.0/codegen/codegen.py | 57 +++++++++++++++++++++++------- gio/gdbus-2.0/codegen/dbustypes.py | 21 +++++++++++ gio/tests/codegen.py | 39 ++++++++++++++++++-- 3 files changed, 102 insertions(+), 15 deletions(-) diff --git a/gio/gdbus-2.0/codegen/codegen.py b/gio/gdbus-2.0/codegen/codegen.py index 7e0a35a01..393328152 100644 --- a/gio/gdbus-2.0/codegen/codegen.py +++ b/gio/gdbus-2.0/codegen/codegen.py @@ -1462,6 +1462,7 @@ class CodeGenerator: self.glib_min_required = glib_min_required self.symbol_decoration_define = symbol_decoration_define self.outfile = outfile + self.marshallers = set() # ---------------------------------------------------------------------------------------------------- @@ -2669,25 +2670,53 @@ class CodeGenerator: "\n" f" {ret_arg.gvalue_set} (return_value, v_return);\n" ) + def generic_marshaller_name(self, args=[], ret=None): + name = "_g_dbus_codegen_marshal_" + name += f"{ret.gvalue_type.upper() if ret else 'VOID'}__" + if args: + name += "_".join(f"{a.gvalue_type.upper()}" for a in args) + else: + name += "VOID" + return name + + def generic_marshaller_name_for_type(self, t): + assert isinstance(t, (dbustypes.Signal, dbustypes.Method)) + + if not t.marshaller_ret_arg: + if not t.marshaller_in_args: + return "g_cclosure_marshal_VOID__VOID" + elif ( + len(t.marshaller_in_args) == 1 + and t.marshaller_in_args[0].gclosure_marshaller + ): + return t.marshaller_in_args[0].gclosure_marshaller + + return self.generic_marshaller_name(t.marshaller_in_args, t.marshaller_ret_arg) + + def generate_generic_marshallers(self, i): + for t in i.signals + i.methods: + marshaller_name = self.generic_marshaller_name_for_type(t) + if marshaller_name.startswith("g_cclosure_"): + self.marshallers.add(marshaller_name) + continue + + if marshaller_name in self.marshallers: + continue + + self.generate_marshaller( + marshaller_name, t.marshaller_in_args, t.marshaller_ret_arg + ) + self.marshallers.add(marshaller_name) + def generate_marshaller_for_type(self, i, t): assert isinstance(t, (dbustypes.Signal, dbustypes.Method)) kind_uscore = utils.camel_case_to_uscore(t.__class__.__name__.lower()) func_name = f"{i.name_lower}_{kind_uscore}_marshal_{t.name_lower}" + marshaller_name = self.generic_marshaller_name_for_type(t) + assert marshaller_name in self.marshallers - if not t.marshaller_ret_arg: - if not t.marshaller_in_args: - self.generate_marshaller_wrapper( - func_name, "g_cclosure_marshal_VOID__VOID" - ) - return - elif len(t.marshaller_in_args) == 1 and t.args[0].gclosure_marshaller: - self.generate_marshaller_wrapper( - func_name, t.args[0].gclosure_marshaller - ) - return - - self.generate_marshaller(func_name, t.marshaller_in_args, t.marshaller_ret_arg) + self.generate_marshaller_wrapper(func_name, marshaller_name) def generate_signal_marshallers(self, i): for s in i.signals: @@ -5419,6 +5448,8 @@ class CodeGenerator: def generate(self): self.generate_body_preamble() + for i in self.ifaces: + self.generate_generic_marshallers(i) for i in self.ifaces: self.generate_interface_intro(i) self.generate_signals_enum_for_interface(i) diff --git a/gio/gdbus-2.0/codegen/dbustypes.py b/gio/gdbus-2.0/codegen/dbustypes.py index 60ba6c4fa..ee11fa2ad 100644 --- a/gio/gdbus-2.0/codegen/dbustypes.py +++ b/gio/gdbus-2.0/codegen/dbustypes.py @@ -83,6 +83,7 @@ class Arg: self.format_in = "@" + self.signature self.format_out = "@" + self.signature self.gvariant_get = "XXX" + self.gvalue_type = "variant" self.gvalue_get = "g_marshal_value_peek_variant" self.gvalue_set = "g_value_take_variant" self.gclosure_marshaller = "g_cclosure_marshal_VOID__VARIANT" @@ -101,6 +102,7 @@ class Arg: self.format_in = "b" self.format_out = "b" self.gvariant_get = "g_variant_get_boolean" + self.gvalue_type = "boolean" self.gvalue_get = "g_marshal_value_peek_boolean" self.gvalue_set = "g_value_set_boolean" self.gclosure_marshaller = "g_cclosure_marshal_VOID__BOOLEAN" @@ -114,6 +116,7 @@ class Arg: self.format_in = "y" self.format_out = "y" self.gvariant_get = "g_variant_get_byte" + self.gvalue_type = "uchar" self.gvalue_get = "g_marshal_value_peek_uchar" self.gvalue_set = "g_value_set_uchar" self.gclosure_marshaller = "g_cclosure_marshal_VOID__UCHAR" @@ -127,6 +130,7 @@ class Arg: self.format_in = "n" self.format_out = "n" self.gvariant_get = "g_variant_get_int16" + self.gvalue_type = "int" self.gvalue_get = "g_marshal_value_peek_int" self.gvalue_set = "g_value_set_int" self.gclosure_marshaller = "g_cclosure_marshal_VOID__INT" @@ -140,6 +144,7 @@ class Arg: self.format_in = "q" self.format_out = "q" self.gvariant_get = "g_variant_get_uint16" + self.gvalue_type = "uint" self.gvalue_get = "g_marshal_value_peek_uint" self.gvalue_set = "g_value_set_uint" self.gclosure_marshaller = "g_cclosure_marshal_VOID__UINT" @@ -153,6 +158,7 @@ class Arg: self.format_in = "i" self.format_out = "i" self.gvariant_get = "g_variant_get_int32" + self.gvalue_type = "int" self.gvalue_get = "g_marshal_value_peek_int" self.gvalue_set = "g_value_set_int" self.gclosure_marshaller = "g_cclosure_marshal_VOID__INT" @@ -166,6 +172,7 @@ class Arg: self.format_in = "u" self.format_out = "u" self.gvariant_get = "g_variant_get_uint32" + self.gvalue_type = "uint" self.gvalue_get = "g_marshal_value_peek_uint" self.gvalue_set = "g_value_set_uint" self.gclosure_marshaller = "g_cclosure_marshal_VOID__UINT" @@ -179,6 +186,7 @@ class Arg: self.format_in = "x" self.format_out = "x" self.gvariant_get = "g_variant_get_int64" + self.gvalue_type = "int64" self.gvalue_get = "g_marshal_value_peek_int64" self.gvalue_set = "g_value_set_int64" self.gclosure_marshaller = None @@ -192,6 +200,7 @@ class Arg: self.format_in = "t" self.format_out = "t" self.gvariant_get = "g_variant_get_uint64" + self.gvalue_type = "uint64" self.gvalue_get = "g_marshal_value_peek_uint64" self.gvalue_set = "g_value_set_uint64" self.gclosure_marshaller = None @@ -205,6 +214,7 @@ class Arg: self.format_in = "d" self.format_out = "d" self.gvariant_get = "g_variant_get_double" + self.gvalue_type = "double" self.gvalue_get = "g_marshal_value_peek_double" self.gvalue_set = "g_value_set_double" self.gclosure_marshaller = "g_cclosure_marshal_VOID__DOUBLE" @@ -219,6 +229,7 @@ class Arg: self.format_in = "s" self.format_out = "s" self.gvariant_get = "g_variant_get_string" + self.gvalue_type = "string" self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" self.gclosure_marshaller = "g_cclosure_marshal_VOID__STRING" @@ -233,6 +244,7 @@ class Arg: self.format_in = "o" self.format_out = "o" self.gvariant_get = "g_variant_get_string" + self.gvalue_type = "string" self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" self.gclosure_marshaller = "g_cclosure_marshal_VOID__STRING" @@ -247,6 +259,7 @@ class Arg: self.format_in = "g" self.format_out = "g" self.gvariant_get = "g_variant_get_string" + self.gvalue_type = "string" self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" self.gclosure_marshaller = "g_cclosure_marshal_VOID__STRING" @@ -261,6 +274,7 @@ class Arg: self.format_in = "^ay" self.format_out = "^ay" self.gvariant_get = "g_variant_get_bytestring" + self.gvalue_type = "string" self.gvalue_get = "g_marshal_value_peek_string" self.gvalue_set = "g_value_set_string" self.gclosure_marshaller = "g_cclosure_marshal_VOID__STRING" @@ -275,6 +289,7 @@ class Arg: self.format_in = "^as" self.format_out = "^as" self.gvariant_get = "g_variant_get_strv" + self.gvalue_type = "boxed" self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" self.gclosure_marshaller = "g_cclosure_marshal_VOID__BOXED" @@ -290,6 +305,7 @@ class Arg: self.format_in = "^ao" self.format_out = "^ao" self.gvariant_get = "g_variant_get_objv" + self.gvalue_type = "boxed" self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" self.gclosure_marshaller = "g_cclosure_marshal_VOID__BOXED" @@ -305,6 +321,7 @@ class Arg: self.format_in = "^aay" self.format_out = "^aay" self.gvariant_get = "g_variant_get_bytestring_array" + self.gvalue_type = "boxed" self.gvalue_get = "g_marshal_value_peek_boxed" self.gvalue_set = "g_value_take_boxed" self.gclosure_marshaller = "g_cclosure_marshal_VOID__BOXED" @@ -375,13 +392,17 @@ class Method: method_invocation_arg = Arg("method_invocation", None) method_invocation_arg.ctype_in = "GDBusMethodInvocation *" + method_invocation_arg.gvalue_type = "object" method_invocation_arg.gvalue_get = "g_marshal_value_peek_object" + method_invocation_arg.gclosure_marshaller = None self.marshaller_in_args = [method_invocation_arg] + self.in_args if self.unix_fd: fd_list_arg = Arg("fd_list", None) fd_list_arg.ctype_in = "GUnixFDList *" + fd_list_arg.gvalue_type = "object" fd_list_arg.gvalue_get = "g_marshal_value_peek_object" + fd_list_arg.gclosure_marshaller = None self.marshaller_in_args.insert(0, fd_list_arg) for a in self.annotations: diff --git a/gio/tests/codegen.py b/gio/tests/codegen.py index 2cdd59b5a..34e7e95d8 100644 --- a/gio/tests/codegen.py +++ b/gio/tests/codegen.py @@ -994,10 +994,20 @@ G_END_DECLS self.assertIs(stripped_out.count(f"{func_name} ("), 1) self.assertIs( - stripped_out.count("g_marshal_value_peek_object (param_values + 1)"), 2 + stripped_out.count("g_marshal_value_peek_object (param_values + 1)"), 1 ) self.assertIs( - stripped_out.count("g_value_set_boolean (return_value, v_return);"), 2 + stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 + ) + + self.assertIs( + stripped_out.count( + "_g_dbus_codegen_marshal_BOOLEAN__OBJECT (\n GClosure" + ), + 1, + ) + self.assertIs( + stripped_out.count("_g_dbus_codegen_marshal_BOOLEAN__OBJECT (closure"), 2 ) @unittest.skipIf(on_win32(), "requires /dev/stdout") @@ -1087,6 +1097,14 @@ G_END_DECLS {''.join(generated_args)} + + {''.join(generated_args)} + + + + + {''.join(generated_args)} + """ @@ -1121,6 +1139,18 @@ G_END_DECLS self.assertIs( stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 ) + func_types = "_".join( + [p["value_type"].upper() for p in self.ARGUMENTS_TYPES.values()] + ) + func_name = f"_g_dbus_codegen_marshal_BOOLEAN__OBJECT_{func_types}" + self.assertIs(stripped_out.count(f"{func_name} (\n GClosure"), 1) + self.assertIs(stripped_out.count(f"{func_name} (closure"), 3) + + func_name = ( + f"org_project_other_callable_iface_method_marshal_method_with_many_args" + ) + self.assertIs(stripped_out.count(f"{func_name},"), 1) + self.assertIs(stripped_out.count(f"{func_name} ("), 1) @unittest.skipIf(on_win32(), "requires /dev/stdout") def test_generate_methods_marshallers_multiple_out_args(self): @@ -1165,6 +1195,11 @@ G_END_DECLS stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1 ) + self.assertIs( + stripped_out.count("_g_dbus_codegen_marshal_BOOLEAN__OBJECT (closure"), + 1, + ) + @unittest.skipIf(on_win32(), "requires /dev/stdout") def test_generate_methods_marshallers_with_unix_fds(self): """Test an interface with `h` arguments"""