GNOME Bugzilla – Bug 316669
Use GErrors instead of throwing errors async
Last modified: 2005-11-01 09:45:36 UTC
When the action performed is synchronous, add GErrors to the API instead of throwing errors through a signal, as it makes it easier for the application to se which call generated which error.
What does this new API look like?
Pretty much like: int btctl_controller_get_signal_strength (BtctlController *bc, const gchar *bdaddr, GError **err); does. Each function that's synchronous and can fail should have a GError in the function call. We need to use g_set_error() in the function if *err is non NULL, and need to create a new quark for the errors, as well as a list of codes for the errors we could get.
What about all errno variants? Is errno propagated through GError?
Using errno is the path to uncomprehensible error messages, and badly targetted ones. So we need to come up with a list of errors we'd like users to see. For example: printf("Failed to connect to SDP server on %s\n", str); would be transformed into something like: BTCTL_ERROR_NO_SDP_SERVER with a message like: "The Bluetooth service daemon isn't available. Make sure your system is correctly setup." The error messages, and error codes need to focus more on the reasons why an error occured in the first place ("No route to host" is useless when trying to contact a Bluetooth device, you wnat to know that you have no Bluetooth adapters...).
Created attachment 53033 [details] [review] btctl_error_quark() function and GError code enum. I see. The errno can provide useful information, e.g. tell the difference between "permission denied" and "device unavailable". But I suppose you're suggesting a kind of translation layer, whereby the errno is translated to one or several BTCTL_ERROR codes? Do we need a mutex lock in btctl_error_quark()?
No, no need for a mutex in btctl_error_quark(). But of course, as you said above we should use the errno to make up decent errors dependant on the context. I just didn't want us to use errnos all round, or match them one for one.
Created attachment 53072 [details] [review] btctl_error_quark() function, GError code enum and some GError FIXMEs fixed. Like this? What to do about the GError in btctl_set_property (see FIXME in patch)? I'm sure the error messages can be improved over time but at least they try to provide useful information.
The creation of the object is not supposed to fail. We check afterwards that the device is initialised. Easiest is to: - create the device using new - in btctl_controller_impl_set_hci_device(), store the error in the object - be able to get the GError from btctl_controller_is_initialised()
Created attachment 53222 [details] [review] btctl_error_quark() function, GError code enum and some GError FIXMEs fixed. Sure. Happy with this? btctl.c | 44 ++++++++++++++++++++++++++++++----- btctl.defs | 3 ++ btctl.h | 13 ++++++++-- btctlimpl.c | 50 +++++++++++++++++++++++----------------- btctlimpl.h | 6 ++-- btlist.c | 7 ++++- 6 files changed, 88 insertions(+), 35 deletions(-)
ping
I'll review it tomorrow, hopefully.
Excellent work. We might need to polish the error messages though. 2005-11-01 Bastien Nocera <hadess@hadess.net> * docs/reference/tmpl/btctl.sgml: * src/btctl.c: (btctl_set_property), (btctl_controller_init), (btctl_error_quark), (btctl_controller_finalize), (btctl_controller_get_discoverable), (btctl_controller_set_discoverable), (btctl_controller_is_initialised): * src/btctl.defs: * src/btctl.h: * src/btctlimpl.c: (btctl_controller_impl_get_discoverable), (btctl_controller_impl_set_discoverable), (btctl_controller_impl_set_hci_device): * src/btctlimpl.h: * src/btlist.c: (main): Patch from Fredrik Noring <noring@nocrew.org> to use GErrors when possible to report errors, instead of throwing errors async (Closes: #316669)