2005-06-26 Colin Walters <walters@verbum.org>

	* glib/dbus-glib.c (dbus_set_g_error): Delete.
	(dbus_g_error_set): New public function from its ashes; used by
	both service-side method implementation and GLib bindings
	internals.
	(dbus_g_error_has_name, dbus_g_error_get_name): New function.
	(_dbus_glib_test): Add some tests.

	* test/glib/test-dbus-glib.c (main): Test dbus_g_error_has_name.

	* test/glib/test-service-glib.c (my_object_throw_error): Use
	dbus_g_error_set.

	* glib/dbus-gobject.c (gerror_to_dbus_error_message): Handle
	errors thrown by dbus_g_error_set.

	* glib/dbus-gmain.c (dbus_g_bus_get): Change to dbus_g_error_set.

	* glib/dbus-gparser.c (validate_signature): Ditto.

	* glib/dbus-gproxy.c (dbus_g_proxy_new_for_name_owner)
	(dbus_g_proxy_end_call_internal): Ditto.

	* glib/Makefile.am: Generate dbus-glib-error-switch.h, which
	converts DBUS_ERROR_x to DBUS_GERROR_x.
	(libdbus_glib_1_la_SOURCES, BUILT_SOURCES, CLEANFILES): Add it.

	* doc/TODO: Remove error TODO.

	* doc/dbus-tutorial.xml: Update with documentation about error
	handling.

	* dbus/make-dbus-glib-error-enum.sh: Tighten up regexp to make
	sure we only change DBUS_ERROR to DBUS_GERROR, not all ERROR to
	GERROR.  Also add DBUS_GERROR_REMOTE_EXCEPTION.

8 files changed, 150 insertions, 22 deletions

diff --git a/glib/.cvsignore b/glib/.cvsignore
index 4e197a9f..91ea2461 100644
--- a/glib/.cvsignore
+++ b/glib/.cvsignore
@@ -6,6 +6,7 @@ Makefile.in
 *.la
 dbus-glib-test
 dbus-binding-tool
+dbus-glib-error-switch.h
 *.bb
 *.bbg
 *.da
diff --git a/glib/Makefile.am b/glib/Makefile.am
index 962cd6f3..26724b93 100644
--- a/glib/Makefile.am
+++ b/glib/Makefile.am
@@ -4,7 +4,15 @@ INCLUDES=-I$(top_srcdir) $(DBUS_CLIENT_CFLAGS) $(DBUS_GLIB_CFLAGS) $(DBUS_GLIB_T
 
 lib_LTLIBRARIES=libdbus-glib-1.la
 
+dbus-glib-error-switch.h: $(top_srcdir)/dbus/dbus-protocol.h make-dbus-glib-error-switch.sh
+	$(srcdir)/make-dbus-glib-error-switch.sh $(top_srcdir)/dbus/dbus-protocol.h $@
+
+BUILT_SOURCES = dbus-glib-error-switch.h
+
+CLEANFILES = $(BUILT_SOURCES)
+
 libdbus_glib_1_la_SOURCES = 			\
+	dbus-glib-error-switch.h		\
 	dbus-glib.c				\
 	dbus-gmain.c				\
 	dbus-gmarshal.c				\
diff --git a/glib/dbus-glib.c b/glib/dbus-glib.c
index 7065b668..3153deef 100644
--- a/glib/dbus-glib.c
+++ b/glib/dbus-glib.c
@@ -26,6 +26,7 @@
 #include <dbus/dbus-glib-lowlevel.h>
 #include "dbus-gtest.h"
 #include "dbus-gutils.h"
+#include <string.h>
 
 #include <libintl.h>
 #define _(x) dgettext (GETTEXT_PACKAGE, x)
@@ -148,26 +149,87 @@ dbus_g_error_quark (void)
   return quark;
 }
 
+#include "dbus-glib-error-switch.h"
 
 /**
- * Set a GError return location from a DBusError.
+ * Set a GError return location from a D-BUS error name and message.
+ * This function should only be used in the implementation of service
+ * methods.
  *
- * @todo expand the DBUS_GERROR enum and take advantage of it here
- * 
  * @param gerror location to store a GError, or #NULL
- * @param derror the DBusError
+ * @param name the D-BUS error name
+ * @param msg the D-BUS error detailed message
  */
 void
-dbus_set_g_error (GError   **gerror,
-                  DBusError *derror)
+dbus_g_error_set (GError    **gerror,
+                  const char *name,
+		  const char *msg)
 {
-  g_return_if_fail (derror != NULL);
-  g_return_if_fail (dbus_error_is_set (derror));
-  
-  g_set_error (gerror, DBUS_GERROR,
-               DBUS_GERROR_FAILED,
-               _("D-BUS error %s: %s"),
-               derror->name, derror->message);  
+  int code;
+  g_return_if_fail (name != NULL);
+  g_return_if_fail (msg != NULL);
+
+  code = dbus_error_to_gerror_code (name);
+  if (code == DBUS_GERROR_REMOTE_EXCEPTION)
+    g_set_error (gerror, DBUS_GERROR,
+		 code,
+		 "%s%c%s",
+		 msg,
+		 '\0',
+		 name);
+  else
+    g_set_error (gerror, DBUS_GERROR,
+		 code,
+		 "%s",
+		 msg);
+}
+
+/**
+ * Determine whether D-BUS error name for a remote exception matches
+ * the given name.  This function is intended to be invoked on a
+ * GError returned from an invocation of a remote method, e.g. via
+ * dbus_g_proxy_end_call.  It will silently return FALSE for errors
+ * which are not remote D-BUS exceptions (i.e. with a domain other
+ * than DBUS_GERROR or a code other than
+ * DBUS_GERROR_REMOTE_EXCEPTION).
+ *
+ * @param gerror the GError given from the remote method
+ * @param name the D-BUS error name
+ * @param msg the D-BUS error detailed message
+ * @returns TRUE iff the remote error has the given name
+ */
+gboolean
+dbus_g_error_has_name (GError *error, const char *name)
+{
+  g_return_val_if_fail (error != NULL, FALSE);
+
+  if (error->domain != DBUS_GERROR
+      || error->code != DBUS_GERROR_REMOTE_EXCEPTION)
+    return FALSE;
+
+  return !strcmp (dbus_g_error_get_name (error), name);
+}
+
+/**
+ * Return the D-BUS name for a remote exception.
+ * This function may only be invoked on a GError returned from an
+ * invocation of a remote method, e.g. via dbus_g_proxy_end_call.
+ * Moreover, you must ensure that the error's domain is DBUS_GERROR,
+ * and the code is DBUS_GERROR_REMOTE_EXCEPTION.
+ *
+ * @param gerror the GError given from the remote method
+ * @param name the D-BUS error name
+ * @param msg the D-BUS error detailed message
+ * @returns the D-BUS error name
+ */
+const char *
+dbus_g_error_get_name (GError *error)
+{
+  g_return_val_if_fail (error != NULL, NULL);
+  g_return_val_if_fail (error->domain == DBUS_GERROR, NULL);
+  g_return_val_if_fail (error->code == DBUS_GERROR_REMOTE_EXCEPTION, NULL);
+
+  return error->message + strlen (error->message) + 1;
 }
 
 /**
@@ -395,7 +457,28 @@ dbus_g_pending_call_cancel (DBusGPendingCall *call)
 gboolean
 _dbus_glib_test (const char *test_data_dir)
 {
+  DBusError err;
+  GError *gerror = NULL;
+
+  dbus_error_init (&err);
+  dbus_set_error_const (&err, DBUS_ERROR_NO_MEMORY, "Out of memory!");
+
+  dbus_g_error_set (&gerror, err.name, err.message);
+  g_assert (gerror != NULL);
+  g_assert (gerror->domain == DBUS_GERROR);
+  g_assert (gerror->code == DBUS_GERROR_NO_MEMORY);
+  g_assert (!strcmp (gerror->message, "Out of memory!"));
   
+  dbus_error_init (&err);
+  g_clear_error (&gerror);
+
+  dbus_g_error_set (&gerror, "com.example.Foo.BlahFailed", "blah failed");
+  g_assert (gerror != NULL);
+  g_assert (gerror->domain == DBUS_GERROR);
+  g_assert (gerror->code == DBUS_GERROR_REMOTE_EXCEPTION);
+  g_assert (dbus_g_error_has_name (gerror, "com.example.Foo.BlahFailed"));
+  g_assert (!strcmp (gerror->message, "blah failed"));
+
   return TRUE;
 }
 
diff --git a/glib/dbus-gmain.c b/glib/dbus-gmain.c
index 08f8aef4..6d3b56c8 100644
--- a/glib/dbus-gmain.c
+++ b/glib/dbus-gmain.c
@@ -720,7 +720,7 @@ dbus_g_bus_get (DBusBusType     type,
   connection = dbus_bus_get (type, &derror);
   if (connection == NULL)
     {
-      dbus_set_g_error (error, &derror);
+      dbus_g_error_set (error, derror.name, derror.message);
       dbus_error_free (&derror);
       return NULL;
     }
diff --git a/glib/dbus-gobject.c b/glib/dbus-gobject.c
index 87926a05..00528906 100644
--- a/glib/dbus-gobject.c
+++ b/glib/dbus-gobject.c
@@ -745,10 +745,17 @@ gerror_to_dbus_error_message (const DBusGObjectInfo *object_info,
     }
   else
     {
-      char *error_name;
-      error_name = gerror_domaincode_to_dbus_error_name (object_info, error->domain, error->code);
-      reply = dbus_message_new_error (message, error_name, error->message);
-      g_free (error_name); 
+      if (error->domain == DBUS_GERROR)
+	reply = dbus_message_new_error (message,
+					dbus_g_error_get_name (error),
+					error->message);
+      else
+	{
+	  char *error_name;
+	  error_name = gerror_domaincode_to_dbus_error_name (object_info, error->domain, error->code);
+	  reply = dbus_message_new_error (message, error_name, error->message);
+	  g_free (error_name); 
+	}
     }
   return reply;
 }
diff --git a/glib/dbus-gparser.c b/glib/dbus-gparser.c
index 745e0dde..90798b19 100644
--- a/glib/dbus-gparser.c
+++ b/glib/dbus-gparser.c
@@ -474,7 +474,7 @@ validate_signature (const char *str,
   
   if (!dbus_signature_validate (str, &derror))
     {
-      dbus_set_g_error (error, &derror);
+      dbus_g_error_set (error, derror.name, derror.message);
       return FALSE;
     }
   return TRUE;
diff --git a/glib/dbus-gproxy.c b/glib/dbus-gproxy.c
index f4fcc31f..3b13ff23 100644
--- a/glib/dbus-gproxy.c
+++ b/glib/dbus-gproxy.c
@@ -1135,7 +1135,7 @@ dbus_g_proxy_new_for_name_owner (DBusGConnection          *connection,
 
  error:
   g_assert (dbus_error_is_set (&derror));
-  dbus_set_g_error (error, &derror);
+  dbus_g_error_set (error, derror.name, derror.message);
   dbus_error_free (&derror);
 
  out:
@@ -1469,14 +1469,14 @@ dbus_g_proxy_end_call_internal (DBusGProxy       *proxy,
       break;
     case DBUS_MESSAGE_TYPE_ERROR:
       dbus_set_error_from_message (&derror, reply);
-      dbus_set_g_error (error, &derror);
+      dbus_g_error_set (error, derror.name, derror.message);
       dbus_error_free (&derror);
       goto out;
       break;
     default:
       dbus_set_error (&derror, DBUS_ERROR_FAILED,
                       "Reply was neither a method return nor an exception");
-      dbus_set_g_error (error, &derror);
+      dbus_g_error_set (error, derror.name, derror.message);
       dbus_error_free (&derror);
       goto out;
       break;
diff --git a/glib/make-dbus-glib-error-switch.sh b/glib/make-dbus-glib-error-switch.sh
new file mode 100755
index 00000000..602cf990
--- /dev/null
+++ b/glib/make-dbus-glib-error-switch.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+SRC=$1
+DEST=$2
+
+die()
+{
+    echo $1 1>&2
+    /bin/rm $DEST.tmp
+    exit 1
+}
+
+echo 'static gint' > $DEST.tmp
+echo 'dbus_error_to_gerror_code (const char *derr)' >> $DEST.tmp
+echo '{' >> $DEST.tmp
+echo '  if (0) ; ' >> $DEST.tmp
+
+cat $SRC | grep '#define DBUS_ERROR' | sed -e 's/#define //g' | \
+  sed -e 's/".*//g' | \
+   (while read line; do \
+     echo '  else if (!strcmp (derr, ' "$line" ' )) '; \
+     echo '    return ' `echo $line | sed -e 's/DBUS_ERROR/DBUS_GERROR/g'` ';'; \
+    done; \
+   ) >> $DEST.tmp
+echo '  else' >> $DEST.tmp
+echo '    return DBUS_GERROR_REMOTE_EXCEPTION;' >> $DEST.tmp
+echo '}' >> $DEST.tmp
+
+mv $DEST.tmp $DEST || die "could not move $DEST.tmp to $DEST"