2005-07-10 Colin Walters <walters@verbum.org>

	* 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.

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  <walters@verbum.org>
+
+	* 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  <walters@verbum.org>
 
 	* 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, "      <arg name=\"%s\" type=\"%s\" direction=\"%s\"/>\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 @@
       <arg type="u" direction="out" />
     </method>
 
+    <method name="IncrementRetval">
+      <arg type="u" name="x" />
+      <arg type="u" direction="out">
+	<annotation name="org.freedesktop.DBus.GLib.ReturnVal" value=""/>
+      </arg>
+    </method>
+
+    <method name="IncrementRetvalError">
+      <arg type="u" name="x" />
+      <arg type="u" direction="out">
+	<annotation name="org.freedesktop.DBus.GLib.ReturnVal" value="error"/>
+      </arg>
+    </method>
+
     <method name="ThrowError">
     </method>