From 8c095eea8fbe2f8c219bdb2aebcf61e4e3993f53 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 10 Jul 2005 22:54:19 +0000 Subject: 2005-07-10 Colin Walters * doc/TODO: Knock off some GLib items with this patch. * glib/dbus-gvalue-utils.c (_dbus_gtype_can_signal_error) (_dbus_gvalue_signals_error): New functions. * glib/dbus-gvalue-utils.h: Prototype them. * glib/dbus-gobject.c (arg_iterate): Update to handle return vals and change to not output const/retval flags for input args. All callers updated. (invoke_object_method): Refactor to handle return values. Add some more comments in various places. Remove debug g_print. * glib/dbus-binding-tool-glib.h (DBUS_GLIB_ANNOTATION_RETURNVAL): New. * glib/dbus-binding-tool-glib.c (dbus_g_type_get_marshal_name): Handle G_TYPE_NONE. (compute_gsignature): New function; refactored from code from compute_marshaller and compute_marshaller_name. Enhance to handle return values and async ops more cleanly. Update for async ops returning NONE instead of BOOLEAN. (compute_marshaller, compute_marshaller_name): Call compute_gsignature and output appropriate string. (generate_glue): Handle return value annotation. Also don't dump constness flag for input arguments. * glib/Makefile.am (DBUS_GLIB_INTERNALS): New variable; contains files shared between installed library and utilities. (libdbus_glib_1_la_SOURCES): Move some stuf into DBUS_GLIB_INTERNALS. (libdbus_gtool_la_SOURCES): Suck in DBUS_GLIB_INTERNALS so the binding tool can access gtype utility functions. * test/glib/test-service-glib.c: * test/glib/test-service-glib.xml: * test/glib/test-dbus-glib.c: Add some tests for return values. --- ChangeLog | 38 ++++ doc/TODO | 4 - glib/Makefile.am | 19 +- glib/dbus-binding-tool-glib.c | 324 +++++++++++++++++++++++----------- glib/dbus-binding-tool-glib.h | 1 + glib/dbus-gobject.c | 242 +++++++++++++++++++------ glib/dbus-gvalue-utils.c | 54 ++++++ glib/dbus-gvalue-utils.h | 4 + glib/examples/statemachine/.cvsignore | 1 + test/glib/test-dbus-glib.c | 41 ++++- test/glib/test-service-glib.c | 46 ++++- test/glib/test-service-glib.xml | 14 ++ 12 files changed, 607 insertions(+), 181 deletions(-) diff --git a/ChangeLog b/ChangeLog index 00c77880..53023ddd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,41 @@ +2005-07-10 Colin Walters + + * doc/TODO: Knock off some GLib items with this patch. + + * glib/dbus-gvalue-utils.c (_dbus_gtype_can_signal_error) + (_dbus_gvalue_signals_error): New functions. + + * glib/dbus-gvalue-utils.h: Prototype them. + + * glib/dbus-gobject.c (arg_iterate): Update to handle return vals + and change to not output const/retval flags for input args. All + callers updated. + (invoke_object_method): Refactor to handle return values. Add + some more comments in various places. Remove debug g_print. + + * glib/dbus-binding-tool-glib.h (DBUS_GLIB_ANNOTATION_RETURNVAL): New. + + * glib/dbus-binding-tool-glib.c (dbus_g_type_get_marshal_name): + Handle G_TYPE_NONE. + (compute_gsignature): New function; refactored from code from + compute_marshaller and compute_marshaller_name. Enhance to + handle return values and async ops more cleanly. Update for + async ops returning NONE instead of BOOLEAN. + (compute_marshaller, compute_marshaller_name): Call compute_gsignature + and output appropriate string. + (generate_glue): Handle return value annotation. Also don't dump + constness flag for input arguments. + + * glib/Makefile.am (DBUS_GLIB_INTERNALS): New variable; contains + files shared between installed library and utilities. + (libdbus_glib_1_la_SOURCES): Move some stuf into DBUS_GLIB_INTERNALS. + (libdbus_gtool_la_SOURCES): Suck in DBUS_GLIB_INTERNALS so the + binding tool can access gtype utility functions. + + * test/glib/test-service-glib.c: + * test/glib/test-service-glib.xml: + * test/glib/test-dbus-glib.c: Add some tests for return values. + 2005-07-09 Colin Walters * glib/dbus-gparser.c (parse_annotation): Add annotations to diff --git a/doc/TODO b/doc/TODO index df0f9c77..38fbe57c 100644 --- a/doc/TODO +++ b/doc/TODO @@ -28,10 +28,6 @@ Important for 1.0 Important for 1.0 GLib Bindings === - - Annotations for "do not take ownership of this return value" on server - - - Support a{sv} - - Test point-to-point mode - Add support for getting sender diff --git a/glib/Makefile.am b/glib/Makefile.am index 26724b93..1abf6805 100644 --- a/glib/Makefile.am +++ b/glib/Makefile.am @@ -11,6 +11,13 @@ BUILT_SOURCES = dbus-glib-error-switch.h CLEANFILES = $(BUILT_SOURCES) +DBUS_GLIB_INTERNALS = \ + dbus-gtype-specialized.c \ + dbus-gutils.c \ + dbus-gutils.h \ + dbus-gvalue-utils.c \ + dbus-gvalue-utils.h + libdbus_glib_1_la_SOURCES = \ dbus-glib-error-switch.h \ dbus-glib.c \ @@ -23,13 +30,9 @@ libdbus_glib_1_la_SOURCES = \ dbus-gtest.c \ dbus-gtest.h \ dbus-gthread.c \ - dbus-gutils.c \ - dbus-gutils.h \ dbus-gvalue.c \ - dbus-gtype-specialized.c \ dbus-gvalue.h \ - dbus-gvalue-utils.c \ - dbus-gvalue-utils.h + $(DBUS_GLIB_INTERNALS) libdbus_glib_HEADERS = \ dbus-gtype-specialized.h @@ -44,14 +47,12 @@ libdbus_glib_1_la_LDFLAGS= -export-symbols-regex "^[^_].*" -version-info $(LT_CU # convenience lib used here and by dbus-viewer noinst_LTLIBRARIES=libdbus-gtool.la -libdbus_gtool_la_SOURCES = \ +libdbus_gtool_la_SOURCES = $(DBUS_GLIB_INTERNALS) \ dbus-gidl.c \ dbus-gidl.h \ dbus-gloader-expat.c \ dbus-gparser.c \ - dbus-gparser.h \ - dbus-gutils.c \ - dbus-gutils.h + dbus-gparser.h libdbus_gtool_la_LIBADD = libdbus-glib-1.la diff --git a/glib/dbus-binding-tool-glib.c b/glib/dbus-binding-tool-glib.c index f161c638..43b23c6d 100644 --- a/glib/dbus-binding-tool-glib.c +++ b/glib/dbus-binding-tool-glib.c @@ -62,6 +62,8 @@ dbus_g_type_get_marshal_name (GType gtype) { switch (G_TYPE_FUNDAMENTAL (gtype)) { + case G_TYPE_NONE: + return "NONE"; case G_TYPE_BOOLEAN: return "BOOLEAN"; case G_TYPE_UCHAR: @@ -112,148 +114,195 @@ dbus_g_type_get_c_name (GType gtype) return g_type_name (gtype); } -static char * -compute_marshaller (MethodInfo *method, GError **error) +static gboolean +compute_gsignature (MethodInfo *method, GType *rettype, GArray **params, GError **error) { GSList *elt; - GString *ret; - gboolean first; + GType retval_type; + GArray *ret; + gboolean is_async; + const char *arg_type; + gboolean retval_signals_error; + + is_async = method_info_get_annotation (method, DBUS_GLIB_ANNOTATION_ASYNC) != NULL; + retval_signals_error = FALSE; - /* All methods required to return boolean for now; - * will be conditional on method info later */ - ret = g_string_new ("BOOLEAN:"); + ret = g_array_new (TRUE, TRUE, sizeof (GType)); - first = TRUE; - /* Append input arguments */ - for (elt = method_info_get_args (method); elt; elt = elt->next) + if (is_async) + retval_type = G_TYPE_NONE; + else { - ArgInfo *arg = elt->data; + gboolean found_retval; - if (arg_info_get_direction (arg) == ARG_IN) + /* Look for return value */ + found_retval = FALSE; + for (elt = method_info_get_args (method); elt; elt = elt->next) { - const char *marshal_name; - GType gtype; - - gtype = dbus_gtype_from_signature (arg_info_get_type (arg), FALSE); - if (gtype == G_TYPE_INVALID) + ArgInfo *arg = elt->data; + const char *returnval_annotation; + + returnval_annotation = arg_info_get_annotation (arg, DBUS_GLIB_ANNOTATION_RETURNVAL); + if (returnval_annotation != NULL) { - g_set_error (error, - DBUS_BINDING_TOOL_ERROR, - DBUS_BINDING_TOOL_ERROR_UNSUPPORTED_CONVERSION, - _("Unsupported conversion from D-BUS type %s to glib-genmarshal type"), - arg_info_get_type (arg)); - g_string_free (ret, TRUE); - return NULL; + arg_type = arg_info_get_type (arg); + retval_type = dbus_gtype_from_signature (arg_type, FALSE); + if (retval_type == G_TYPE_INVALID) + goto invalid_type; + found_retval = TRUE; + if (!strcmp (returnval_annotation, "error")) + retval_signals_error = TRUE; + break; } - - marshal_name = dbus_g_type_get_marshal_name (gtype); - g_assert (marshal_name); - - if (!first) - g_string_append (ret, ","); - else - first = FALSE; - g_string_append (ret, marshal_name); + } + if (!found_retval) + { + retval_type = G_TYPE_BOOLEAN; + retval_signals_error = TRUE; } } - if (method_info_get_annotation (method, DBUS_GLIB_ANNOTATION_ASYNC) != NULL) + *rettype = retval_type; + + /* Handle all input arguments */ + for (elt = method_info_get_args (method); elt; elt = elt->next) { - if (!first) - g_string_append (ret, ","); - g_string_append (ret, "POINTER"); - first = FALSE; + ArgInfo *arg = elt->data; + if (arg_info_get_direction (arg) == ARG_IN) + { + GType gtype; + + arg_type = arg_info_get_type (arg); + gtype = dbus_gtype_from_signature (arg_type, FALSE); + if (gtype == G_TYPE_INVALID) + goto invalid_type; + + g_array_append_val (ret, gtype); + } } - else + + if (!is_async) { /* Append pointer for each out arg storage */ for (elt = method_info_get_args (method); elt; elt = elt->next) { ArgInfo *arg = elt->data; + /* Skip return value */ + if (arg_info_get_annotation (arg, DBUS_GLIB_ANNOTATION_RETURNVAL) != NULL) + continue; + if (arg_info_get_direction (arg) == ARG_OUT) { - if (!first) - g_string_append (ret, ","); - else - first = FALSE; - g_string_append (ret, "POINTER"); + GType gtype; + arg_type = arg_info_get_type (arg); + gtype = dbus_gtype_from_signature (arg_type, FALSE); + if (gtype == G_TYPE_INVALID) + goto invalid_type; + /* We actually just need a pointer for the return value + storage */ + gtype = G_TYPE_POINTER; + g_array_append_val (ret, gtype); } } - /* Final GError parameter */ - if (!first) - g_string_append (ret, ","); - g_string_append (ret, "POINTER"); + if (retval_signals_error) + { + /* Final GError parameter */ + GType gtype = G_TYPE_POINTER; + g_array_append_val (ret, gtype); + } + } + else + { + /* Context pointer */ + GType gtype = G_TYPE_POINTER; + g_array_append_val (ret, gtype); } - return g_string_free (ret, FALSE); + *params = ret; + return TRUE; + invalid_type: + g_set_error (error, + DBUS_BINDING_TOOL_ERROR, + DBUS_BINDING_TOOL_ERROR_UNSUPPORTED_CONVERSION, + _("Unsupported conversion from D-BUS type %s to glib-genmarshal type"), + arg_type); + return FALSE; } + static char * -compute_marshaller_name (MethodInfo *method, const char *prefix, GError **error) +compute_marshaller (MethodInfo *method, GError **error) { - GSList *elt; + GArray *signature; + GType rettype; + const char *marshal_name; GString *ret; + guint i; - /* All methods required to return boolean for now; - * will be conditional on method info later */ - ret = g_string_new (MARSHAL_PREFIX); - g_string_append (ret, prefix); - g_string_append (ret, "_BOOLEAN_"); + if (!compute_gsignature (method, &rettype, &signature, error)) + return NULL; - /* Append input arguments */ - for (elt = method_info_get_args (method); elt; elt = elt->next) + ret = g_string_new (""); + marshal_name = dbus_g_type_get_marshal_name (rettype); + g_assert (marshal_name != NULL); + g_string_append (ret, marshal_name); + g_string_append_c (ret, ':'); + for (i = 0; i < signature->len; i++) { - ArgInfo *arg = elt->data; + marshal_name = dbus_g_type_get_marshal_name (g_array_index (signature, GType, i)); + g_assert (marshal_name != NULL); + g_string_append (ret, marshal_name); + if (i < signature->len - 1) + g_string_append_c (ret, ','); + } + if (signature->len == 0) + { + marshal_name = dbus_g_type_get_marshal_name (G_TYPE_NONE); + g_assert (marshal_name != NULL); + g_string_append (ret, marshal_name); + } + g_array_free (signature, TRUE); + return g_string_free (ret, FALSE); +} - if (arg_info_get_direction (arg) == ARG_IN) - { - const char *marshal_name; - const char *type; - GType gtype; +static char * +compute_marshaller_name (MethodInfo *method, const char *prefix, GError **error) +{ + GString *ret; + GArray *signature; + GType rettype; + const char *marshal_name; + guint i; - type = arg_info_get_type (arg); - gtype = dbus_gtype_from_signature (type, FALSE); - if (gtype == G_TYPE_INVALID) - { - g_set_error (error, - DBUS_BINDING_TOOL_ERROR, - DBUS_BINDING_TOOL_ERROR_UNSUPPORTED_CONVERSION, - _("Unsupported conversion from D-BUS type %s to glib type"), - type); - g_string_free (ret, TRUE); - return NULL; - } - marshal_name = dbus_g_type_get_marshal_name (gtype); - g_assert (marshal_name != NULL); + if (!compute_gsignature (method, &rettype, &signature, error)) + return NULL; - g_string_append (ret, "_"); - g_string_append (ret, marshal_name); - } - } + ret = g_string_new (MARSHAL_PREFIX); + g_string_append (ret, prefix); + g_string_append_c (ret, '_'); - if (method_info_get_annotation (method, DBUS_GLIB_ANNOTATION_ASYNC) != NULL) + marshal_name = dbus_g_type_get_marshal_name (rettype); + g_assert (marshal_name != NULL); + g_string_append (ret, marshal_name); + g_string_append (ret, "__"); + for (i = 0; i < signature->len; i++) { - g_string_append (ret, "_POINTER"); + marshal_name = dbus_g_type_get_marshal_name (g_array_index (signature, GType, i)); + g_assert (marshal_name != NULL); + g_string_append (ret, marshal_name); + if (i < signature->len - 1) + g_string_append_c (ret, '_'); } - else + if (signature->len == 0) { - /* Append pointer for each out arg storage */ - for (elt = method_info_get_args (method); elt; elt = elt->next) - { - ArgInfo *arg = elt->data; - - if (arg_info_get_direction (arg) == ARG_OUT) - { - g_string_append (ret, "_POINTER"); - } - } - /* Final GError parameter */ - g_string_append (ret, "_POINTER"); + marshal_name = dbus_g_type_get_marshal_name (G_TYPE_NONE); + g_assert (marshal_name != NULL); + g_string_append (ret, marshal_name); } - + g_array_free (signature, TRUE); return g_string_free (ret, FALSE); } @@ -482,6 +531,7 @@ generate_glue (BaseInfo *base, DBusBindingToolCData *data, GError **error) char *method_c_name; gboolean async = FALSE; GSList *args; + gboolean found_retval = FALSE; method = (MethodInfo *) tmp->data; method_c_name = g_strdup (method_info_get_annotation (method, DBUS_GLIB_ANNOTATION_C_SYMBOL)); @@ -531,6 +581,7 @@ generate_glue (BaseInfo *base, DBusBindingToolCData *data, GError **error) { ArgInfo *arg; char direction; + const char *returnval_annotation; arg = args->data; @@ -561,17 +612,86 @@ generate_glue (BaseInfo *base, DBusBindingToolCData *data, GError **error) g_set_error (error, DBUS_BINDING_TOOL_ERROR, DBUS_BINDING_TOOL_ERROR_INVALID_ANNOTATION, - "Input argument \"%s\" has const annotation in method \"%s\" of interface \"%s\"\n", + "Input argument \"%s\" cannot have const annotation in method \"%s\" of interface \"%s\"\n", arg_info_get_name (arg), method_info_get_name (method), interface_info_get_name (interface)); return FALSE; } g_string_append_c (object_introspection_data_blob, 'C'); + g_string_append_c (object_introspection_data_blob, '\0'); + } + else if (arg_info_get_direction (arg) == ARG_OUT) + { + g_string_append_c (object_introspection_data_blob, 'F'); + g_string_append_c (object_introspection_data_blob, '\0'); + } + + returnval_annotation = arg_info_get_annotation (arg, DBUS_GLIB_ANNOTATION_RETURNVAL); + if (returnval_annotation != NULL) + { + GType gtype; + + if (found_retval) + { + g_set_error (error, + DBUS_BINDING_TOOL_ERROR, + DBUS_BINDING_TOOL_ERROR_INVALID_ANNOTATION, + "Multiple arguments with return value annotation in method \"%s\" of interface \"%s\"\n", + method_info_get_name (method), + interface_info_get_name (interface)); + return FALSE; + } + found_retval = TRUE; + if (arg_info_get_direction (arg) == ARG_IN) + { + g_set_error (error, + DBUS_BINDING_TOOL_ERROR, + DBUS_BINDING_TOOL_ERROR_INVALID_ANNOTATION, + "Input argument \"%s\" cannot have return value annotation in method \"%s\" of interface \"%s\"\n", + arg_info_get_name (arg), + method_info_get_name (method), + interface_info_get_name (interface)); + return FALSE; + } + if (!strcmp ("", returnval_annotation)) + g_string_append_c (object_introspection_data_blob, 'R'); + else if (!strcmp ("error", returnval_annotation)) + { + gtype = dbus_gtype_from_signature (arg_info_get_type (arg), TRUE); + if (!_dbus_gtype_can_signal_error (gtype)) + { + g_set_error (error, + DBUS_BINDING_TOOL_ERROR, + DBUS_BINDING_TOOL_ERROR_INVALID_ANNOTATION, + "Output argument \"%s\" cannot signal error with type \"%s\" in method \"%s\" of interface \"%s\"\n", + arg_info_get_name (arg), + g_type_name (gtype), + method_info_get_name (method), + interface_info_get_name (interface)); + return FALSE; + } + g_string_append_c (object_introspection_data_blob, 'E'); + } + else + { + g_set_error (error, + DBUS_BINDING_TOOL_ERROR, + DBUS_BINDING_TOOL_ERROR_INVALID_ANNOTATION, + "Invalid ReturnVal annotation for argument \"%s\" in method \"%s\" of interface \"%s\"\n", + arg_info_get_name (arg), + method_info_get_name (method), + interface_info_get_name (interface)); + return FALSE; + } + + g_string_append_c (object_introspection_data_blob, '\0'); + } + else if (arg_info_get_direction (arg) == ARG_OUT) + { + g_string_append_c (object_introspection_data_blob, 'N'); + g_string_append_c (object_introspection_data_blob, '\0'); } - else - g_string_append_c (object_introspection_data_blob, 'F'); - g_string_append_c (object_introspection_data_blob, '\0'); g_string_append (object_introspection_data_blob, arg_info_get_type (arg)); g_string_append_c (object_introspection_data_blob, '\0'); diff --git a/glib/dbus-binding-tool-glib.h b/glib/dbus-binding-tool-glib.h index 62ad2b79..bbb54f72 100644 --- a/glib/dbus-binding-tool-glib.h +++ b/glib/dbus-binding-tool-glib.h @@ -28,6 +28,7 @@ G_BEGIN_DECLS #define DBUS_GLIB_ANNOTATION_C_SYMBOL "org.freedesktop.DBus.GLib.CSymbol" #define DBUS_GLIB_ANNOTATION_ASYNC "org.freedesktop.DBus.GLib.Async" #define DBUS_GLIB_ANNOTATION_CONST "org.freedesktop.DBus.GLib.Const" +#define DBUS_GLIB_ANNOTATION_RETURNVAL "org.freedesktop.DBus.GLib.ReturnVal" gboolean dbus_binding_tool_output_glib_client (BaseInfo *info, GIOChannel *channel, gboolean ignore_unsupported, GError **error); gboolean dbus_binding_tool_output_glib_server (BaseInfo *info, GIOChannel *channel, const char *prefix, GError **error); diff --git a/glib/dbus-gobject.c b/glib/dbus-gobject.c index 58291573..1dcf4bb9 100644 --- a/glib/dbus-gobject.c +++ b/glib/dbus-gobject.c @@ -143,45 +143,91 @@ method_arg_info_from_object_info (const DBusGObjectInfo *object, return string_table_lookup (get_method_data (object, method), 3);/*RB was 2*/ } +typedef enum +{ + RETVAL_NONE, + RETVAL_NOERROR, + RETVAL_ERROR +} RetvalType; + static const char * arg_iterate (const char *data, const char **name, gboolean *in, gboolean *constval, + RetvalType *retval, const char **type) { - *name = data; + gboolean inarg; + + if (name) + *name = data; data = string_table_next (data); switch (*data) { case 'I': - *in = TRUE; + inarg = TRUE; break; case 'O': - *in = FALSE; + inarg = FALSE; break; default: g_warning ("invalid arg direction '%c'", *data); + inarg = FALSE; break; } + if (in) + *in = inarg; - data = string_table_next (data); - switch (*data) + if (!inarg) { - case 'F': - *constval = FALSE; - break; - case 'C': - *constval = TRUE; - break; - default: - g_warning ("invalid arg const value '%c'", *data); - break; + data = string_table_next (data); + switch (*data) + { + case 'F': + if (constval) + *constval = FALSE; + break; + case 'C': + if (constval) + *constval = TRUE; + break; + default: + g_warning ("invalid arg const value '%c'", *data); + break; + } + data = string_table_next (data); + switch (*data) + { + case 'N': + if (retval) + *retval = RETVAL_NONE; + break; + case 'E': + if (retval) + *retval = RETVAL_ERROR; + break; + case 'R': + if (retval) + *retval = RETVAL_NOERROR; + break; + default: + g_warning ("invalid arg ret value '%c'", *data); + break; + } + } + else + { + if (constval) + *constval = FALSE; + if (retval) + *retval = FALSE; } data = string_table_next (data); - *type = data; + if (type) + *type = data; return string_table_next (data); } @@ -202,10 +248,9 @@ method_dir_signature_from_object_info (const DBusGObjectInfo *object, { const char *name; gboolean arg_in; - gboolean constval; const char *type; - arg = arg_iterate (arg, &name, &arg_in, &constval, &type); + arg = arg_iterate (arg, &name, &arg_in, NULL, NULL, &type); if (arg_in == in) g_string_append (ret, type); @@ -340,10 +385,9 @@ write_interface (gpointer key, gpointer val, gpointer user_data) { const char *name; gboolean arg_in; - gboolean constval; const char *type; - args = arg_iterate (args, &name, &arg_in, &constval, &type); + args = arg_iterate (args, &name, &arg_in, NULL, NULL, &type); /* FIXME - handle container types */ g_string_append_printf (xml, " \n", @@ -846,15 +890,17 @@ invoke_object_method (GObject *object, GValue return_value = {0,}; GClosure closure; char *in_signature; - char *out_signature = NULL; - int current_type; - DBusSignatureIter out_signature_iter; GArray *out_param_values = NULL; GValueArray *out_param_gvalues = NULL; int out_param_count; int out_param_pos, out_param_gvalue_pos; DBusHandlerResult result; DBusMessage *reply; + gboolean have_retval; + gboolean retval_signals_error; + gboolean retval_is_synthetic; + gboolean retval_is_constant; + const char *arg_metadata; gerror = NULL; @@ -866,6 +912,11 @@ invoke_object_method (GObject *object, else call_only = FALSE; + have_retval = FALSE; + retval_signals_error = FALSE; + retval_is_synthetic = FALSE; + retval_is_constant = FALSE; + /* This is evil. We do this to work around the fact that * the generated glib marshallers check a flag in the closure object * which we don't care about. We don't need/want to create @@ -924,21 +975,72 @@ invoke_object_method (GObject *object, } else { - out_signature = method_output_signature_from_object_info (object_info, method); + RetvalType retval; + gboolean arg_in; + gboolean arg_const; + const char *argsig; - /* Count number of output parameters */ - dbus_signature_iter_init (&out_signature_iter, out_signature); + arg_metadata = method_arg_info_from_object_info (object_info, method); + + /* Count number of output parameters, and look for a return value */ out_param_count = 0; - while ((current_type = dbus_signature_iter_get_current_type (&out_signature_iter)) != DBUS_TYPE_INVALID) + while (*arg_metadata) + { + arg_metadata = arg_iterate (arg_metadata, NULL, &arg_in, &arg_const, &retval, &argsig); + if (arg_in) + continue; + if (retval != RETVAL_NONE) + { + DBusSignatureIter tmp_sigiter; + /* This is the function return value */ + g_assert (!have_retval); + have_retval = TRUE; + retval_is_synthetic = FALSE; + + switch (retval) + { + case RETVAL_NONE: + g_assert_not_reached (); + break; + case RETVAL_NOERROR: + retval_signals_error = FALSE; + break; + case RETVAL_ERROR: + retval_signals_error = TRUE; + break; + } + + retval_is_constant = arg_const; + + /* Initialize our return GValue with the specified type */ + dbus_signature_iter_init (&tmp_sigiter, argsig); + g_value_init (&return_value, dbus_gtype_from_signature_iter (&tmp_sigiter, FALSE)); + } + else + { + /* It's a regular output value */ + out_param_count++; + } + } + + /* For compatibility, if we haven't found a return value, we assume + * the function returns a gboolean for signalling an error + * (and therefore also takes a GError). We also note that it + * is a "synthetic" return value; i.e. we aren't going to be + * sending it over the bus, it's just to signal an error. + */ + if (!have_retval) { - out_param_count++; - dbus_signature_iter_next (&out_signature_iter); + have_retval = TRUE; + retval_is_synthetic = TRUE; + retval_signals_error = TRUE; + g_value_init (&return_value, G_TYPE_BOOLEAN); } - /* Create an array to store the actual values of OUT - * parameters. Then, create a GValue boxed POINTER - * to each of those values, and append to the invocation, - * so the method can return the OUT parameters. + /* Create an array to store the actual values of OUT parameters + * (other than the real function return, if any). Then, create + * a GValue boxed POINTER to each of those values, and append to + * the invocation, so the method can return the OUT parameters. */ out_param_values = g_array_sized_new (FALSE, TRUE, sizeof (GTypeCValue), out_param_count); @@ -948,16 +1050,32 @@ invoke_object_method (GObject *object, out_param_gvalues = g_value_array_new (out_param_count); out_param_pos = 0; out_param_gvalue_pos = 0; - dbus_signature_iter_init (&out_signature_iter, out_signature); - while ((current_type = dbus_signature_iter_get_current_type (&out_signature_iter)) != DBUS_TYPE_INVALID) + + /* Reset argument metadata pointer */ + arg_metadata = method_arg_info_from_object_info (object_info, method); + + /* Iterate over output arguments again, this time allocating space for + * them as appopriate. + */ + while (*arg_metadata) { GValue value = {0, }; GTypeCValue storage; + DBusSignatureIter tmp_sigiter; + GType current_gtype; + + arg_metadata = arg_iterate (arg_metadata, NULL, &arg_in, NULL, &retval, &argsig); + /* Skip over input arguments and the return value, if any */ + if (arg_in || retval != RETVAL_NONE) + continue; + + dbus_signature_iter_init (&tmp_sigiter, argsig); + current_gtype = dbus_gtype_from_signature_iter (&tmp_sigiter, FALSE); g_value_init (&value, G_TYPE_POINTER); /* We special case variants to make method invocation a bit nicer */ - if (current_type != DBUS_TYPE_VARIANT) + if (current_gtype != G_TYPE_VALUE) { memset (&storage, 0, sizeof (storage)); g_array_append_val (out_param_values, storage); @@ -971,17 +1089,20 @@ invoke_object_method (GObject *object, out_param_gvalue_pos++; } g_value_array_append (value_array, &value); - dbus_signature_iter_next (&out_signature_iter); } + } - /* Append GError as final argument */ + /* Append GError as final argument if necessary */ + if (retval_signals_error) + { + g_assert (have_retval); g_value_array_append (value_array, NULL); g_value_init (g_value_array_get_nth (value_array, value_array->n_values - 1), G_TYPE_POINTER); g_value_set_pointer (g_value_array_get_nth (value_array, value_array->n_values - 1), &gerror); } + /* Actually invoke method */ - g_value_init (&return_value, G_TYPE_BOOLEAN); - method->marshaller (&closure, &return_value, + method->marshaller (&closure, have_retval ? &return_value : NULL, value_array->n_values, value_array->values, NULL, method->function); @@ -990,22 +1111,35 @@ invoke_object_method (GObject *object, result = DBUS_HANDLER_RESULT_HANDLED; goto done; } - had_error = !g_value_get_boolean (&return_value); + if (retval_signals_error) + had_error = _dbus_gvalue_signals_error (&return_value); + else + had_error = FALSE; if (!had_error) { DBusMessageIter iter; - const char *arg_metadata; - - /* Grab the argument metadata and iterate over it */ - arg_metadata = method_arg_info_from_object_info (object_info, method); reply = dbus_message_new_method_return (message); if (reply == NULL) goto nomem; - /* Append OUT arguments to reply */ + /* Append output arguments to reply */ dbus_message_iter_init_append (reply, &iter); + + /* First, append the return value, unless it's synthetic */ + if (have_retval && !retval_is_synthetic) + { + if (!dbus_gvalue_marshal (&iter, &return_value)) + goto nomem; + if (!retval_is_constant) + g_value_unset (&return_value); + } + + /* Grab the argument metadata and iterate over it */ + arg_metadata = method_arg_info_from_object_info (object_info, method); + + /* Now append any remaining return values */ out_param_pos = 0; out_param_gvalue_pos = 0; while (*arg_metadata) @@ -1014,26 +1148,26 @@ invoke_object_method (GObject *object, const char *arg_name; gboolean arg_in; gboolean constval; + RetvalType retval; const char *arg_signature; DBusSignatureIter argsigiter; do { - /* Look for constness; skip over input arguments */ - arg_metadata = arg_iterate (arg_metadata, &arg_name, &arg_in, &constval, &arg_signature); + /* Iterate over only output values; skip over input + arguments and the return value */ + arg_metadata = arg_iterate (arg_metadata, &arg_name, &arg_in, &constval, &retval, &arg_signature); } - while (arg_in && *arg_metadata); + while ((arg_in || retval != RETVAL_NONE) && *arg_metadata); - /* If the last argument we saw was input, we must be done iterating over - * output arguments. + /* If the last argument we saw was input or the return + * value, we must be done iterating over output arguments. */ - if (arg_in) + if (arg_in || retval != RETVAL_NONE) break; dbus_signature_iter_init (&argsigiter, arg_signature); - g_print ("looking at arg %s (%s)\n", arg_name, constval ? "TRUE" : "FALSE"); - g_value_init (&gvalue, dbus_gtype_from_signature_iter (&argsigiter, FALSE)); if (G_VALUE_TYPE (&gvalue) != G_TYPE_VALUE) { @@ -1070,14 +1204,12 @@ invoke_object_method (GObject *object, result = DBUS_HANDLER_RESULT_HANDLED; done: g_free (in_signature); - g_free (out_signature); if (!call_only) { g_array_free (out_param_values, TRUE); g_value_array_free (out_param_gvalues); } g_value_array_free (value_array); - g_value_unset (&return_value); return result; nomem: result = DBUS_HANDLER_RESULT_NEED_MEMORY; diff --git a/glib/dbus-gvalue-utils.c b/glib/dbus-gvalue-utils.c index b17eee16..af2fff19 100644 --- a/glib/dbus-gvalue-utils.c +++ b/glib/dbus-gvalue-utils.c @@ -206,6 +206,58 @@ dbus_gvalue_take (GValue *value, return TRUE; } +gboolean +_dbus_gtype_can_signal_error (GType gtype) +{ + switch (gtype) + { + case G_TYPE_BOOLEAN: + case G_TYPE_INT: + case G_TYPE_UINT: + case G_TYPE_STRING: + case G_TYPE_BOXED: + case G_TYPE_OBJECT: + return TRUE; + default: + return FALSE; + } +} + +gboolean +_dbus_gvalue_signals_error (const GValue *value) +{ + /* Hardcoded rules for return value semantics for certain + * types. Perhaps in the future we'd want an annotation + * specifying which return values are errors, but in + * reality people will probably just use boolean and + * boxed, and there the semantics are pretty standard. + */ + switch (G_TYPE_FUNDAMENTAL (G_VALUE_TYPE (value))) + { + case G_TYPE_BOOLEAN: + return (g_value_get_boolean (value) == FALSE); + break; + case G_TYPE_INT: + return (g_value_get_int (value) < 0); + break; + case G_TYPE_UINT: + return (g_value_get_uint (value) == 0); + break; + case G_TYPE_STRING: + return (g_value_get_string (value) == NULL); + break; + case G_TYPE_BOXED: + return (g_value_get_boxed (value) == NULL); + break; + case G_TYPE_OBJECT: + return (g_value_get_boxed (value) == NULL); + break; + default: + g_assert_not_reached (); + } +} + + static gboolean hash_func_from_gtype (GType gtype, GHashFunc *func) { @@ -1061,4 +1113,6 @@ _dbus_gvalue_utils_test (const char *datadir) return TRUE; } + + #endif /* DBUS_BUILD_TESTS */ diff --git a/glib/dbus-gvalue-utils.h b/glib/dbus-gvalue-utils.h index 551e4289..4f81081e 100644 --- a/glib/dbus-gvalue-utils.h +++ b/glib/dbus-gvalue-utils.h @@ -64,6 +64,10 @@ gboolean dbus_gvalue_store (GValue *value, gboolean dbus_gvalue_take (GValue *value, GTypeCValue *cvalue); +gboolean _dbus_gtype_can_signal_error (GType gtype); +gboolean _dbus_gvalue_signals_error (const GValue *value); + + G_END_DECLS #endif diff --git a/glib/examples/statemachine/.cvsignore b/glib/examples/statemachine/.cvsignore index b1f9981d..6bf74218 100644 --- a/glib/examples/statemachine/.cvsignore +++ b/glib/examples/statemachine/.cvsignore @@ -7,6 +7,7 @@ Makefile.in statemachine-client statemachine-server statemachine-glue.h +statemachine-server-glue.h run-with-tmp-session-bus.conf sm-marshal.[ch] *.bb diff --git a/test/glib/test-dbus-glib.c b/test/glib/test-dbus-glib.c index 4fbe797e..786235de 100644 --- a/test/glib/test-dbus-glib.c +++ b/test/glib/test-dbus-glib.c @@ -504,11 +504,11 @@ main (int argc, char **argv) G_TYPE_UINT, &v_UINT32_2, G_TYPE_INVALID)) lose_gerror ("Failed to complete Increment call", error); - g_assert (n_times_echo_cb_entered == 1); - if (v_UINT32_2 != 43) lose ("Increment call returned %d, should be 43", v_UINT32_2); + v_UINT32_2 = 0; + g_print ("Calling Increment (async)\n"); call = dbus_g_proxy_begin_call (proxy, "Increment", increment_received_cb, g_strdup ("moo"), g_free, G_TYPE_UINT, 42, @@ -517,6 +517,30 @@ main (int argc, char **argv) exit_timeout = g_timeout_add (5000, timed_exit, loop); g_main_loop_run (loop); + g_print ("Calling IncrementRetval\n"); + error = NULL; + v_UINT32_2 = 0; + if (!dbus_g_proxy_call (proxy, "IncrementRetval", &error, + G_TYPE_UINT, 42, + G_TYPE_INVALID, + G_TYPE_UINT, &v_UINT32_2, + G_TYPE_INVALID)) + lose_gerror ("Failed to complete Increment call", error); + if (v_UINT32_2 != 43) + lose ("IncrementRetval call returned %d, should be 43", v_UINT32_2); + + g_print ("Calling IncrementRetvalError\n"); + error = NULL; + v_UINT32_2 = 0; + if (!dbus_g_proxy_call (proxy, "IncrementRetvalError", &error, + G_TYPE_UINT, 5, + G_TYPE_INVALID, + G_TYPE_UINT, &v_UINT32_2, + G_TYPE_INVALID)) + lose_gerror ("Failed to complete Increment call", error); + if (v_UINT32_2 != 6) + lose ("IncrementRetval call returned %d, should be 6", v_UINT32_2); + g_print ("Calling ThrowError\n"); if (dbus_g_proxy_call (proxy, "ThrowError", &error, G_TYPE_INVALID, G_TYPE_INVALID) != FALSE) @@ -529,6 +553,19 @@ main (int argc, char **argv) g_print ("ThrowError failed (as expected) returned error: %s\n", error->message); g_clear_error (&error); + g_print ("Calling IncrementRetvalError (for error)\n"); + error = NULL; + v_UINT32_2 = 0; + if (dbus_g_proxy_call (proxy, "IncrementRetvalError", &error, + G_TYPE_UINT, 20, + G_TYPE_INVALID, + G_TYPE_UINT, &v_UINT32_2, + G_TYPE_INVALID) != FALSE) + lose ("IncrementRetvalError call unexpectedly succeeded!"); + if (!dbus_g_error_has_name (error, "org.freedesktop.DBus.Tests.MyObject.Foo")) + lose ("IncrementRetvalError call returned unexpected error \"%s\": %s", dbus_g_error_get_name (error), error->message); + g_clear_error (&error); + error = NULL; g_print ("Calling Uppercase\n"); if (!dbus_g_proxy_call (proxy, "Uppercase", &error, diff --git a/test/glib/test-service-glib.c b/test/glib/test-service-glib.c index 853b401a..e44310f3 100644 --- a/test/glib/test-service-glib.c +++ b/test/glib/test-service-glib.c @@ -52,6 +52,10 @@ gboolean my_object_do_nothing (MyObject *obj, GError **error); gboolean my_object_increment (MyObject *obj, gint32 x, gint32 *ret, GError **error); +gint32 my_object_increment_retval (MyObject *obj, gint32 x); + +gint32 my_object_increment_retval_error (MyObject *obj, gint32 x, GError **error); + gboolean my_object_throw_error (MyObject *obj, GError **error); gboolean my_object_uppercase (MyObject *obj, const char *str, char **ret, GError **error); @@ -91,9 +95,9 @@ gboolean my_object_emit_frobnicate (MyObject *obj, GError **error); gboolean my_object_terminate (MyObject *obj, GError **error); -gboolean my_object_async_increment (MyObject *obj, gint32 x, DBusGMethodInvocation *context); +void my_object_async_increment (MyObject *obj, gint32 x, DBusGMethodInvocation *context); -gboolean my_object_async_throw_error (MyObject *obj, DBusGMethodInvocation *context); +void my_object_async_throw_error (MyObject *obj, DBusGMethodInvocation *context); #include "test-service-glib-glue.h" @@ -283,6 +287,27 @@ my_object_increment (MyObject *obj, gint32 x, gint32 *ret, GError **error) return TRUE; } +gint32 +my_object_increment_retval (MyObject *obj, gint32 x) +{ + return x + 1; +} + +gint32 +my_object_increment_retval_error (MyObject *obj, gint32 x, GError **error) +{ + if (x + 1 > 10) + { + g_set_error (error, + MY_OBJECT_ERROR, + MY_OBJECT_ERROR_FOO, + "%s", + "x is bigger than 9"); + return FALSE; + } + return x + 1; +} + gboolean my_object_throw_error (MyObject *obj, GError **error) { @@ -559,14 +584,13 @@ do_async_increment (IncrementData *data) return FALSE; } -gboolean +void my_object_async_increment (MyObject *obj, gint32 x, DBusGMethodInvocation *context) { IncrementData *data = g_new0 (IncrementData, 1); data->x = x; data->context = context; g_idle_add ((GSourceFunc)do_async_increment, data); - return TRUE; } static gboolean @@ -582,13 +606,12 @@ do_async_error (IncrementData *data) return FALSE; } -gboolean +void my_object_async_throw_error (MyObject *obj, DBusGMethodInvocation *context) { IncrementData *data = g_new0(IncrementData, 1); data->context = context; g_idle_add ((GSourceFunc)do_async_error, data); - return TRUE; } @@ -623,11 +646,16 @@ main (int argc, char **argv) g_printerr ("Launching test-service-glib\n"); - g_log_set_always_fatal (G_LOG_LEVEL_CRITICAL); - g_log_set_always_fatal (G_LOG_LEVEL_WARNING); - loop = g_main_loop_new (NULL, FALSE); + { + GLogLevelFlags fatal_mask; + + fatal_mask = g_log_set_always_fatal (G_LOG_FATAL_MASK); + fatal_mask |= G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL; + g_log_set_always_fatal (fatal_mask); + } + error = NULL; connection = dbus_g_bus_get (DBUS_BUS_STARTER, &error); diff --git a/test/glib/test-service-glib.xml b/test/glib/test-service-glib.xml index 84524a10..4ea2b2bb 100644 --- a/test/glib/test-service-glib.xml +++ b/test/glib/test-service-glib.xml @@ -10,6 +10,20 @@ + + + + + + + + + + + + + + -- cgit