From df901b528bc1e1edd96e9e91b94c9c9b795b8ffd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 26 Jun 2005 17:02:09 +0000 Subject: 2005-06-26 Colin Walters * 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. --- ChangeLog | 37 ++++++++++++ dbus/dbus-glib.h | 7 +++ dbus/make-dbus-glib-error-enum.sh | 3 +- doc/TODO | 2 - doc/dbus-tutorial.xml | 102 ++++++++++++++++++++++----------- glib/.cvsignore | 1 + glib/Makefile.am | 8 +++ glib/dbus-glib.c | 109 +++++++++++++++++++++++++++++++----- glib/dbus-gmain.c | 2 +- glib/dbus-gobject.c | 15 +++-- glib/dbus-gparser.c | 2 +- glib/dbus-gproxy.c | 6 +- glib/make-dbus-glib-error-switch.sh | 29 ++++++++++ test/glib/test-dbus-glib.c | 2 + test/glib/test-service-glib.c | 7 +-- 15 files changed, 269 insertions(+), 63 deletions(-) create mode 100755 glib/make-dbus-glib-error-switch.sh diff --git a/ChangeLog b/ChangeLog index ea8779aa..d7ec76da 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,40 @@ +2005-06-26 Colin Walters + + * 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 Patch from Ross Burton diff --git a/dbus/dbus-glib.h b/dbus/dbus-glib.h index 2d5005bb..901b5720 100644 --- a/dbus/dbus-glib.h +++ b/dbus/dbus-glib.h @@ -81,6 +81,13 @@ typedef enum #include } 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); diff --git a/dbus/make-dbus-glib-error-enum.sh b/dbus/make-dbus-glib-error-enum.sh index c2f538db..55362ded 100755 --- a/dbus/make-dbus-glib-error-enum.sh +++ b/dbus/make-dbus-glib-error-enum.sh @@ -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 diff --git a/doc/TODO b/doc/TODO index 4b953bad..cfaae877 100644 --- 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 diff --git a/doc/dbus-tutorial.xml b/doc/dbus-tutorial.xml index 304d0d70..bd6313c7 100644 --- a/doc/dbus-tutorial.xml +++ b/doc/dbus-tutorial.xml @@ -772,8 +772,13 @@ main (int argc, char **argv) if (!dbus_g_proxy_call (proxy, "ListNames", &error, G_TYPE_INVALID, G_TYPE_STRV, &name_list, G_TYPE_INVALID)) { - g_printerr ("Failed to complete ListNames call: %s\n", - error->message); + /* Just do demonstrate remote exceptions versus regular GError */ + if (error->domain == DBUS_GERROR && error->code == DBUS_GERROR_REMOTE_EXCEPTION) + g_printerr ("Caught remote method exception %s: %s", + dbus_g_error_get_name (error), + error->message); + else + g_printerr ("Error: %s\n", error->message); g_error_free (error); exit (1); } @@ -830,10 +835,60 @@ main (int argc, char **argv) You may connect to signals using dbus_g_proxy_add_signal and - dbus_g_proxy_connect_signal. At the - moment, dbus_g_proxy_add_signal requires - the D-BUS types of the remote object; this will likely be - changed later. + dbus_g_proxy_connect_signal. You must + invoke dbus_g_proxy_add_signal to specify + the signature of your signal handlers; you may then invoke + dbus_g_proxy_connect_signal multiple times. + + + 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 + glib-genmarshal, and then register + it using dbus_g_object_register_marshaller. + + + + Error handling and remote exceptions + + All of the GLib binding methods such as + dbus_g_proxy_end_call return a + GError. This GError can + represent two different things: + + + + 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 + DBUS_GERROR, and a corresponding code + such as DBUS_GERROR_NO_MEMORY. It will + not be typical for applications to handle these errors + specifically. + + + + + 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 GError, but some + special rules apply. + + + The set error will have the domain + DBUS_GERROR as above, and will also + have the code + DBUS_GERROR_REMOTE_EXCEPTION. In order + to access the remote exception name, you must use a + special accessor, such as + dbus_g_error_has_name or + dbus_g_error_get_name. The remote + exception detailed message is accessible via the regular + GError message member. + + + @@ -850,10 +905,7 @@ main (int argc, char **argv) G_TYPE_INVALID, DBUS_TYPE_G_UCHAR_ARRAY, &arr, G_TYPE_INVALID)) { - g_printerr ("Failed to complete Foobar: %s\n", - error->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, &ret, G_TYPE_INVALID)) { - g_printerr ("Failed to complete HashSize: %s\n", - error->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, &strret, G_TYPE_INVALID)) { - g_printerr ("Failed to complete GetStuff: %s\n", - error->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->message); - g_error_free (error); - exit (1); + /* Handle error */ } g_strfreev (strs_dynamic); @@ -958,10 +1001,7 @@ main (int argc, char **argv) G_TYPE_STRV, &strs, G_TYPE_INVALID)) { - g_printerr ("Failed to complete GetStrs: %s\n", - error->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, &val, G_TYPE_INVALID, G_TYPE_INVALID)) { - g_printerr ("Failed to complete SendVariant: %s\n", - error->message); - g_error_free (error); - exit (1); + /* Handle error */ } g_assert (ret == 2); g_value_unset (&val); @@ -1003,10 +1040,7 @@ main (int argc, char **argv) if (!dbus_g_proxy_call (proxy, "GetVariant", &error, G_TYPE_INVALID, G_TYPE_VALUE, &val, G_TYPE_INVALID)) { - g_printerr ("Failed to complete GetVariant: %s\n", - error->message); - g_error_free (error); - exit (1); + /* Handle error */ } if (G_VALUE_TYPE (&val) == G_TYPE_STRING) printf ("%s\n", g_value_get_string (&val)); 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 #include "dbus-gtest.h" #include "dbus-gutils.h" +#include #include #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" diff --git a/test/glib/test-dbus-glib.c b/test/glib/test-dbus-glib.c index ba979cb6..94aeaeb3 100644 --- a/test/glib/test-dbus-glib.c +++ b/test/glib/test-dbus-glib.c @@ -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); diff --git a/test/glib/test-service-glib.c b/test/glib/test-service-glib.c index 6089367e..21c4458d 100644 --- a/test/glib/test-service-glib.c +++ b/test/glib/test-service-glib.c @@ -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; } -- cgit