2005-06-26 Colin Walters <walters@verbum.org>
authorColin Walters <walters@verbum.org>
Sun, 26 Jun 2005 17:02:09 +0000 (17:02 +0000)
committerColin Walters <walters@verbum.org>
Sun, 26 Jun 2005 17:02:09 +0000 (17:02 +0000)
* 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.

15 files changed:
ChangeLog
dbus/dbus-glib.h
dbus/make-dbus-glib-error-enum.sh
doc/TODO
doc/dbus-tutorial.xml
glib/.cvsignore
glib/Makefile.am
glib/dbus-glib.c
glib/dbus-gmain.c
glib/dbus-gobject.c
glib/dbus-gparser.c
glib/dbus-gproxy.c
glib/make-dbus-glib-error-switch.sh [new file with mode: 0755]
test/glib/test-dbus-glib.c
test/glib/test-service-glib.c

index ea8779a..d7ec76d 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,40 @@
+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.
+
 2005-06-22  Colin Walters  <walters@verbum.org>
        
        Patch from Ross Burton <ross@openedhand.com>
index 2d5005b..901b572 100644 (file)
@@ -81,6 +81,13 @@ typedef enum
 #include <dbus/dbus-glib-error-enum.h>    
 } DBusGError;
 
+void             dbus_g_error_set         (GError     **error,
+                                          const char  *name,
+                                          const char  *msg);
+gboolean         dbus_g_error_has_name    (GError      *error,
+                                          const char  *name);
+const char *     dbus_g_error_get_name    (GError      *error);
+
 void             dbus_g_thread_init (void);
 DBusGConnection* dbus_g_bus_get     (DBusBusType   type,
                                      GError      **error);
index c2f538d..55362de 100755 (executable)
@@ -11,11 +11,12 @@ die()
 }
 
 cat $SRC | grep '#define DBUS_ERROR' | sed -e 's/#define //g' | \
-  sed -e 's/".*//g' | sed -e 's/_ERROR/_GERROR/g' | sed -e 's/ *$/,/g' > $DEST.tmp
+  sed -e 's/".*//g' | sed -e 's/DBUS_ERROR/DBUS_GERROR/g' | sed -e 's/ *$/,/g' > $DEST.tmp
 
 if ! test -s $DEST.tmp ; then
     die "$DEST.tmp is empty, something went wrong, see any preceding error message"
 fi
+echo "DBUS_GERROR_REMOTE_EXCEPTION" >> $DEST.tmp
 
 echo "#ifndef DBUS_INSIDE_DBUS_GLIB_H" >> $DEST.tmp
 echo '#error "' "$DEST" 'may only be included by dbus-glib.h"' >> $DEST.tmp
index 4b953ba..cfaae87 100644 (file)
--- a/doc/TODO
+++ b/doc/TODO
@@ -33,8 +33,6 @@ Important for 1.0 GLib Bindings
  - Fix TYPE_OBJECT_PATH marshalling; see:
    http://lists.freedesktop.org/archives/dbus/2005-June/002774.html
 
- - Fix errors - need to get specific error back, not UnmappedError crap
-
  - DBusGProxy doesn't emit "destroy" when it should
 
 Might as Well for 1.0
index 304d0d7..bd6313c 100644 (file)
@@ -772,8 +772,13 @@ main (int argc, char **argv)
   if (!dbus_g_proxy_call (proxy, "ListNames", &amp;error, G_TYPE_INVALID,
                           G_TYPE_STRV, &amp;name_list, G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete ListNames call: %s\n",
-                  error-&gt;message);
+      /* Just do demonstrate remote exceptions versus regular GError */
+      if (error->domain == DBUS_GERROR &amp;&amp; error->code == DBUS_GERROR_REMOTE_EXCEPTION)
+        g_printerr ("Caught remote method exception %s: %s",
+                   dbus_g_error_get_name (error),
+                   error-&gt;message);
+      else
+        g_printerr ("Error: %s\n", error-&gt;message);
       g_error_free (error);
       exit (1);
     }
@@ -830,10 +835,60 @@ main (int argc, char **argv)
       <para>
        You may connect to signals using
        <literal>dbus_g_proxy_add_signal</literal> and
-       <literal>dbus_g_proxy_connect_signal</literal>.  At the
-       moment, <literal>dbus_g_proxy_add_signal</literal> requires
-       the D-BUS types of the remote object; this will likely be
-       changed later.
+       <literal>dbus_g_proxy_connect_signal</literal>.  You must
+       invoke <literal>dbus_g_proxy_add_signal</literal> to specify
+       the signature of your signal handlers; you may then invoke
+       <literal>dbus_g_proxy_connect_signal</literal> multiple times.
+      </para>
+      <para>
+       Note that it will often be the case that there is no builtin
+       marshaller for the type signature of a remote signal.  In that
+       case, you must generate a marshaller yourself by using
+       <application>glib-genmarshal</application>, and then register
+       it using <literal>dbus_g_object_register_marshaller</literal>.
+      </para>
+    </sect2>
+    <sect2 id="glib-error-handling">
+      <title>Error handling and remote exceptions</title>
+      <para>
+       All of the GLib binding methods such as
+       <literal>dbus_g_proxy_end_call</literal> return a
+       <literal>GError</literal>.  This <literal>GError</literal> can
+       represent two different things:
+      <itemizedlist>
+       <listitem>
+         <para>
+           An internal D-BUS error, such as an out-of-memory
+           condition, an I/O error, or a network timeout.  Errors
+           generated by the D-BUS library itself have the domain
+           <literal>DBUS_GERROR</literal>, and a corresponding code
+           such as <literal>DBUS_GERROR_NO_MEMORY</literal>.  It will
+           not be typical for applications to handle these errors
+           specifically.
+         </para>
+       </listitem>
+       <listitem>
+         <para>
+           A remote D-BUS exception, thrown by the peer, bus, or
+           service.  D-BUS remote exceptions have both a textual
+           "name" and a "message".  The GLib bindings store this
+           information in the <literal>GError</literal>, but some
+           special rules apply.
+         </para>
+         <para>
+           The set error will have the domain
+           <literal>DBUS_GERROR</literal> as above, and will also
+           have the code
+           <literal>DBUS_GERROR_REMOTE_EXCEPTION</literal>.  In order
+           to access the remote exception name, you must use a
+           special accessor, such as
+           <literal>dbus_g_error_has_name</literal> or
+           <literal>dbus_g_error_get_name</literal>.  The remote
+           exception detailed message is accessible via the regular
+           GError <literal>message</literal> member.
+         </para>
+       </listitem>
+      </itemizedlist>
       </para>
     </sect2>
     <sect2 id="glib-more-examples">
@@ -850,10 +905,7 @@ main (int argc, char **argv)
                          G_TYPE_INVALID,
                          DBUS_TYPE_G_UCHAR_ARRAY, &amp;arr, G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete Foobar: %s\n",
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
+      /* Handle error */
     }
    g_assert (arr != NULL);
    printf ("got back %u values", arr->len);
@@ -875,10 +927,7 @@ main (int argc, char **argv)
                           DBUS_TYPE_G_STRING_STRING_HASH, hash, G_TYPE_INVALID,
                          G_TYPE_UINT, &amp;ret, G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete HashSize: %s\n",
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
+      /* Handle error */
     }
   g_assert (ret == 2);
   g_hash_table_destroy (hash);
@@ -899,10 +948,7 @@ main (int argc, char **argv)
                           G_TYPE_STRING, &amp;strret,
                          G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete GetStuff: %s\n",
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
+      /* Handle error */
     }
   printf ("%s %s", boolret ? "TRUE" : "FALSE", strret);
   g_free (strret);
@@ -933,10 +979,7 @@ main (int argc, char **argv)
                          G_TYPE_INVALID,
                          G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete TwoStrArrays: %s\n",
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
+      /* Handle error */
     }
    g_strfreev (strs_dynamic);
 </programlisting>
@@ -958,10 +1001,7 @@ main (int argc, char **argv)
                           G_TYPE_STRV, &amp;strs,
                          G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete GetStrs: %s\n",
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
+      /* Handle error */
     }
    for (strs_p = strs; *strs_p; strs_p++)
      printf ("got string: \"%s\"", *strs_p);
@@ -983,10 +1023,7 @@ main (int argc, char **argv)
                           G_TYPE_VALUE, &amp;val, G_TYPE_INVALID,
                          G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete SendVariant: %s\n",
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
+      /* Handle error */
     }
   g_assert (ret == 2);
   g_value_unset (&amp;val);
@@ -1003,10 +1040,7 @@ main (int argc, char **argv)
   if (!dbus_g_proxy_call (proxy, "GetVariant", &amp;error, G_TYPE_INVALID,
                           G_TYPE_VALUE, &amp;val, G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete GetVariant: %s\n",
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
+      /* Handle error */
     }
   if (G_VALUE_TYPE (&amp;val) == G_TYPE_STRING)
     printf ("%s\n", g_value_get_string (&amp;val));
index 4e197a9..91ea246 100644 (file)
@@ -6,6 +6,7 @@ Makefile.in
 *.la
 dbus-glib-test
 dbus-binding-tool
+dbus-glib-error-switch.h
 *.bb
 *.bbg
 *.da
index 962cd6f..26724b9 100644 (file)
@@ -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                         \
index 7065b66..3153dee 100644 (file)
@@ -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;
 }
 
index 08f8aef..6d3b56c 100644 (file)
@@ -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;
     }
index 87926a0..0052890 100644 (file)
@@ -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;
 }
index 745e0dd..90798b1 100644 (file)
@@ -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;
index f4fcc31..3b13ff2 100644 (file)
@@ -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 (executable)
index 0000000..602cf99
--- /dev/null
@@ -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"
index ba979cb..94aeaeb 100644 (file)
@@ -318,6 +318,8 @@ main (int argc, char **argv)
   error = NULL;
   if (dbus_g_proxy_end_call (proxy, call, &error, G_TYPE_INVALID) != FALSE)
     lose ("ThrowError call unexpectedly succeeded!");
+  if (!dbus_g_error_has_name (error, "org.freedesktop.DBus.Tests.MyObject.Foo"))
+    lose ("ThrowError call returned unexpected error %s", dbus_g_error_get_name (error));
 
   g_print ("ThrowError failed (as expected) returned error: %s\n", error->message);
   g_clear_error (&error);
index 6089367..21c4458 100644 (file)
@@ -237,10 +237,9 @@ my_object_increment (MyObject *obj, gint32 x, gint32 *ret, GError **error)
 gboolean
 my_object_throw_error (MyObject *obj, GError **error)
 {
-  g_set_error (error,
-              MY_OBJECT_ERROR,
-              MY_OBJECT_ERROR_FOO,
-              "this method always loses");
+  dbus_g_error_set (error,
+                   "org.freedesktop.DBus.Tests.MyObject.Foo",
+                   "this method always loses");    
   return FALSE;
 }