[RFC obexd v0 0/2] D-Bus error from GError

linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [RFC obexd v0 0/2] D-Bus error from GError
@ 2012-05-03  9:44 Mikel Astiz
  2012-05-03  9:44 ` [RFC obexd v0 1/2] gdbus: Add helper function for error-handling Mikel Astiz
  2012-05-03  9:44 ` [RFC obexd v0 2/2] client: Use new D-Bus helper function Mikel Astiz
  0 siblings, 2 replies; 4+ messages in thread
From: Mikel Astiz @ 2012-05-03  9:44 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

It is common that a D-Bus error message must be create from a GError object. Often this also involves freeing the GError's memory, which leads to boilerplate code. This RFC proposes a helper function in gdbus in order to remove this code.

The proposed name is g_dbus_steal_from_gerror(), which creates a DBusMessage out of a GError object, automatically frees the memory, and sets the pointer to NULL.

This removes 75 error-handling lines from the obex-client codebase, as included in patch 2/2. There are some other occurrences in the server side and also in BlueZ, but these have not been included here.

As returning GError objects starts to be more common in internal APIs (as planned for obex-client), this helper function would be even more convenient.

Mikel Astiz (2):
  gdbus: Add helper function for error-handling
  client: Use new D-Bus helper function

 client/ftp.c     |   85 +++++++++++++++--------------------------------------
 client/manager.c |   12 ++-----
 client/map.c     |   31 ++++++--------------
 client/pbap.c    |   29 +++++-------------
 client/sync.c    |   20 ++++---------
 gdbus/gdbus.h    |    4 ++
 gdbus/object.c   |   11 +++++++
 7 files changed, 66 insertions(+), 126 deletions(-)

-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [RFC obexd v0 1/2] gdbus: Add helper function for error-handling
  2012-05-03  9:44 [RFC obexd v0 0/2] D-Bus error from GError Mikel Astiz
@ 2012-05-03  9:44 ` Mikel Astiz
  2012-05-03 11:23   ` Johan Hedberg
  2012-05-03  9:44 ` [RFC obexd v0 2/2] client: Use new D-Bus helper function Mikel Astiz
  1 sibling, 1 reply; 4+ messages in thread
From: Mikel Astiz @ 2012-05-03  9:44 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

The new function is a convenient way to create a D-Bus error message and
automatically free the GError object.
---
 gdbus/gdbus.h  |    4 ++++
 gdbus/object.c |   11 +++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index a0583e6..485ee01 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -138,6 +138,10 @@ DBusMessage *g_dbus_create_error(DBusMessage *message, const char *name,
 					__attribute__((format(printf, 3, 4)));
 DBusMessage *g_dbus_create_error_valist(DBusMessage *message, const char *name,
 					const char *format, va_list args);
+
+DBusMessage *g_dbus_steal_from_gerror(DBusMessage *message, const char *name,
+						GError **err);
+
 DBusMessage *g_dbus_create_reply(DBusMessage *message, int type, ...);
 DBusMessage *g_dbus_create_reply_valist(DBusMessage *message,
 						int type, va_list args);
diff --git a/gdbus/object.c b/gdbus/object.c
index 8bc12f5..c6e5550 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -765,6 +765,17 @@ DBusMessage *g_dbus_create_error(DBusMessage *message, const char *name,
 	return reply;
 }
 
+DBusMessage *g_dbus_steal_from_gerror(DBusMessage *message, const char *name,
+								GError **err)
+{
+	DBusMessage *reply;
+
+	reply = g_dbus_create_error(message, name, "%s", (*err)->message);
+	g_clear_error(err);
+
+	return reply;
+}
+
 DBusMessage *g_dbus_create_reply_valist(DBusMessage *message,
 						int type, va_list args)
 {
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC obexd v0 1/2] gdbus: Add helper function for error-handling
  2012-05-03  9:44 ` [RFC obexd v0 1/2] gdbus: Add helper function for error-handling Mikel Astiz
@ 2012-05-03 11:23   ` Johan Hedberg
  0 siblings, 0 replies; 4+ messages in thread
From: Johan Hedberg @ 2012-05-03 11:23 UTC (permalink / raw)
  To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz

Hi Mikel,

On Thu, May 03, 2012, Mikel Astiz wrote:
> +DBusMessage *g_dbus_steal_from_gerror(DBusMessage *message, const char *name,
> +								GError **err)
> +{
> +	DBusMessage *reply;
> +
> +	reply = g_dbus_create_error(message, name, "%s", (*err)->message);
> +	g_clear_error(err);
> +
> +	return reply;
> +}

Did you consider following the semantics of dbus_set_error_from_message?

The calling code could then look something like:

	if (g_dbus_steal_from_gerror(&gerr, "foo.bar.baz", &reply))
		return reply;

	do other stuff since there was no error...

One thing that bothers me a bit is that you're forcing the use of a
single generic D-Bus error code for what may be a multitude of various
GError codes/domains (the only inherited part being the error message).
It'd be nice if we could somehow provide a mapping table from GError
codes/domains to D-Bus errors and only in the no-match case fall back to
a generic "org.bluez.Error.Failed".

Johan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [RFC obexd v0 2/2] client: Use new D-Bus helper function
  2012-05-03  9:44 [RFC obexd v0 0/2] D-Bus error from GError Mikel Astiz
  2012-05-03  9:44 ` [RFC obexd v0 1/2] gdbus: Add helper function for error-handling Mikel Astiz
@ 2012-05-03  9:44 ` Mikel Astiz
  1 sibling, 0 replies; 4+ messages in thread
From: Mikel Astiz @ 2012-05-03  9:44 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

This refactoring makes use of the recently introduced helper function to
remove boilerplate error-handling code.
---
 client/ftp.c     |   85 +++++++++++++++--------------------------------------
 client/manager.c |   12 ++-----
 client/map.c     |   31 ++++++--------------
 client/pbap.c    |   29 +++++-------------
 client/sync.c    |   20 ++++---------
 5 files changed, 51 insertions(+), 126 deletions(-)

diff --git a/client/ftp.c b/client/ftp.c
index 0e6af47..7607107 100644
--- a/client/ftp.c
+++ b/client/ftp.c
@@ -80,14 +80,9 @@ static DBusMessage *change_folder(DBusConnection *connection,
 				"org.openobex.Error.InvalidArguments", NULL);
 
 	obc_session_setpath(session, folder, async_cb, message, &err);
-	if (err != NULL) {
-		DBusMessage *reply;
-		reply =  g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 
 	dbus_message_ref(message);
 
@@ -236,14 +231,9 @@ static DBusMessage *create_folder(DBusConnection *connection,
 				"org.openobex.Error.InvalidArguments", NULL);
 
 	obc_session_mkdir(session, folder, async_cb, message, &err);
-	if (err != NULL) {
-		DBusMessage *reply;
-		reply = g_dbus_create_error(message,
-				"org.openobex.Error.Failed",
-				"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 
 	dbus_message_ref(message);
 
@@ -259,13 +249,9 @@ static DBusMessage *list_folder(DBusConnection *connection,
 
 	obc_session_get(session, "x-obex/folder-listing", NULL, NULL,
 				NULL, 0, list_folder_callback, message, &err);
-	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 
 	dbus_message_ref(message);
 
@@ -289,13 +275,9 @@ static DBusMessage *get_file(DBusConnection *connection,
 
 	obc_session_get(session, NULL, source_file, target_file, NULL, 0,
 					get_file_callback, message, &err);
-	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 
 	dbus_message_ref(message);
 
@@ -319,13 +301,9 @@ static DBusMessage *put_file(DBusConnection *connection,
 				"Invalid arguments in method call");
 
 	obc_session_send(session, sourcefile, targetfile, &err);
-	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 
 	return dbus_message_new_method_return(message);
 }
@@ -346,14 +324,9 @@ static DBusMessage *copy_file(DBusConnection *connection,
 				"org.openobex.Error.InvalidArguments", NULL);
 
 	obc_session_copy(session, filename, destname, async_cb, message, &err);
-	if (err != NULL) {
-		DBusMessage *reply;
-		reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 
 	dbus_message_ref(message);
 
@@ -376,14 +349,9 @@ static DBusMessage *move_file(DBusConnection *connection,
 				"org.openobex.Error.InvalidArguments", NULL);
 
 	obc_session_move(session, filename, destname, async_cb, message, &err);
-	if (err != NULL) {
-		DBusMessage *reply;
-		reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 
 	dbus_message_ref(message);
 
@@ -405,14 +373,9 @@ static DBusMessage *delete(DBusConnection *connection,
 				"org.openobex.Error.InvalidArguments", NULL);
 
 	obc_session_delete(session, file, async_cb, message, &err);
-	if (err != NULL) {
-		DBusMessage *reply;
-		reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 
 	dbus_message_ref(message);
 
diff --git a/client/manager.c b/client/manager.c
index 6d08702..1447a09 100644
--- a/client/manager.c
+++ b/client/manager.c
@@ -287,10 +287,8 @@ static void pull_obc_session_callback(struct obc_session *session,
 	obc_session_pull(session, "text/x-vcard", data->filename,
 					pull_complete_callback, data, &gerr);
 	if (gerr != NULL) {
-		reply = g_dbus_create_error(data->message,
-						"org.openobex.Error.Failed",
-						"%s", gerr->message);
-		g_error_free(gerr);
+		reply = g_dbus_steal_from_gerror(data->message,
+					"org.openobex.Error.Failed", &gerr);
 		goto fail;
 	}
 
@@ -504,10 +502,8 @@ static void capability_obc_session_callback(struct obc_session *session,
 	obc_session_pull(session, "x-obex/capability", NULL,
 				capabilities_complete_callback, data, &gerr);
 	if (gerr != NULL) {
-		reply = g_dbus_create_error(data->message,
-					"org.openobex.Error.Failed",
-					"%s", gerr->message);
-		g_error_free(gerr);
+		reply = g_dbus_steal_from_gerror(data->message,
+					"org.openobex.Error.Failed", &gerr);
 		goto fail;
 	}
 
diff --git a/client/map.c b/client/map.c
index 1b4e404..4be73e4 100644
--- a/client/map.c
+++ b/client/map.c
@@ -80,14 +80,9 @@ static DBusMessage *map_setpath(DBusConnection *connection,
 					NULL);
 
 	obc_session_setpath(map->session, folder, simple_cb, map, &err);
-	if (err != NULL) {
-		DBusMessage *reply;
-		reply =  g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					"org.openobex.Error.Failed", &err);
 
 	map->msg = dbus_message_ref(message);
 
@@ -137,13 +132,9 @@ static DBusMessage *map_get_folder_listing(DBusConnection *connection,
 	obc_session_get(map->session, "x-obex/folder-listing", NULL,
 							NULL, NULL, 0,
 							buffer_cb, map, &err);
-	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					"org.openobex.Error.Failed", &err);
 
 	map->msg = dbus_message_ref(message);
 
@@ -169,13 +160,9 @@ static DBusMessage *map_get_message_listing(DBusConnection *connection,
 	obc_session_get(map->session, "x-bt/MAP-msg-listing", folder,
 							NULL, NULL, 0,
 							buffer_cb, map, &err);
-	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+					"org.openobex.Error.Failed", &err);
 
 	map->msg = dbus_message_ref(message);
 
diff --git a/client/pbap.c b/client/pbap.c
index baf2ca6..ccca69c 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -491,12 +491,9 @@ static DBusMessage *pull_phonebook(struct pbap_data *pbap,
 				(guint8 *) &apparam, sizeof(apparam),
 				func, request, &err);
 	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
 		pending_request_free(request);
-		return reply;
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 	}
 
 	return NULL;
@@ -557,13 +554,9 @@ static DBusMessage *pull_vcard_listing(struct pbap_data *pbap,
 				pull_vcard_listing_callback, request, &err);
 	g_free(apparam);
 	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
 		pending_request_free(request);
-		return reply;
-
+		return g_dbus_steal_from_gerror(message,
+					 "org.openobex.Error.Failed", &err);
 	}
 
 	return NULL;
@@ -713,13 +706,10 @@ static DBusMessage *pbap_select(DBusConnection *connection,
 	obc_session_setpath(pbap->session, path, pbap_setpath_cb, request,
 									&err);
 	if (err != NULL) {
-		DBusMessage *reply;
-		reply =  g_dbus_create_error(message, ERROR_INF ".Failed",
-							"%s", err->message);
-		g_error_free(err);
 		g_free(path);
 		pending_request_free(request);
-		return reply;
+		return g_dbus_steal_from_gerror(message,
+						 ERROR_INF ".Failed", &err);
 	}
 
 	g_free(pbap->path);
@@ -781,12 +771,9 @@ static DBusMessage *pbap_pull_vcard(DBusConnection *connection,
 				(guint8 *)&apparam, sizeof(apparam),
 				pull_phonebook_callback, request, &err);
 	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						"org.openobex.Error.Failed",
-						"%s", err->message);
-		g_error_free(err);
 		pending_request_free(request);
-		return reply;
+		return g_dbus_steal_from_gerror(message,
+					"org.openobex.Error.Failed", &err);
 	}
 
 	return NULL;
diff --git a/client/sync.c b/client/sync.c
index 9a26f5b..3ef4e0a 100644
--- a/client/sync.c
+++ b/client/sync.c
@@ -139,13 +139,9 @@ static DBusMessage *sync_getphonebook(DBusConnection *connection,
 					NULL, NULL, 0,
 					sync_getphonebook_callback, sync,
 					&err);
-	if (err != 0) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						ERROR_INF ".Failed",
-						err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != 0)
+		return g_dbus_steal_from_gerror(message,
+						ERROR_INF ".Failed", &err);
 
 	sync->msg = dbus_message_ref(message);
 
@@ -171,13 +167,9 @@ static DBusMessage *sync_putphonebook(DBusConnection *connection,
 
 	obc_session_put(sync->session, buf, strlen(buf), sync->phonebook_path,
 									&err);
-	if (err != NULL) {
-		DBusMessage *reply = g_dbus_create_error(message,
-						ERROR_INF ".Failed",
-						err->message);
-		g_error_free(err);
-		return reply;
-	}
+	if (err != NULL)
+		return g_dbus_steal_from_gerror(message,
+						ERROR_INF ".Failed", &err);
 
 	return dbus_message_new_method_return(message);
 }
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-05-03 11:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-03  9:44 [RFC obexd v0 0/2] D-Bus error from GError Mikel Astiz
2012-05-03  9:44 ` [RFC obexd v0 1/2] gdbus: Add helper function for error-handling Mikel Astiz
2012-05-03 11:23   ` Johan Hedberg
2012-05-03  9:44 ` [RFC obexd v0 2/2] client: Use new D-Bus helper function Mikel Astiz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).