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] 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 = """