[RFC] Add adapter session request/release to obex-client

[RFC] Add adapter session request/release to obex-client

[Dmitriy Paliy]

Hi,

This set of patches adds system bus connection to obex-client and
handling of adapter's method calls, among others, as
RequestSession and ReleaseSession.

Such ensures OBEX transfers done within a single adapter session,
which in turn significantly simplifies file transfer API for clients
of obex-client.

Any comments would be appreciated.

BR,
Dmitriy

[PATCH 1/6] Add system bus connection in obex-client

[Dmitriy Paliy]

Connection to system bus is added in obex-client. Purpose is to carry
out OBEX transfers within a single adapter's session.
---
 client/session.c |    9 +++++++++
 client/session.h |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/client/session.c b/client/session.c
index 266d00c..4a4922f 100644
--- a/client/session.c
+++ b/client/session.c
@@ -201,6 +201,9 @@ static void session_free(struct session_data *session)
 		dbus_connection_unref(session->conn);
 	}
 
+	if (session->conn_system)
+		dbus_connection_unref(session->conn_system);
+
 	sessions = g_slist_remove(sessions, session);
 
 	g_free(session->callback);
@@ -565,6 +568,12 @@ struct session_data *session_create(const char *source,
 		return NULL;
 	}
 
+	session->conn_system = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
+	if (session->conn_system == NULL) {
+		session_free(session);
+		return NULL;
+	}
+
 	if (source == NULL)
 		bacpy(&session->src, BDADDR_ANY);
 	else
diff --git a/client/session.h b/client/session.h
index 6f8a434..554b494 100644
--- a/client/session.h
+++ b/client/session.h
@@ -42,6 +42,7 @@ struct session_data {
 	uuid_t uuid;		/* Bluetooth Service Class */
 	gchar *path;		/* Session path */
 	DBusConnection *conn;
+	DBusConnection *conn_system; /* system bus connection */
 	DBusMessage *msg;
 	GwObex *obex;
 	GIOChannel *io;
-- 
1.7.4.1

[PATCH 2/6] Split up session_create in separate functions

[Dmitriy Paliy]

Connection of RFCOMM and SDP are extracted from session_create function
into session_connect. Such allows making asynchronous calls before
creating connections.
---
 client/session.c |   41 ++++++++++++++++++++++++-----------------
 1 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/client/session.c b/client/session.c
index 4a4922f..4016e35 100644
--- a/client/session.c
+++ b/client/session.c
@@ -534,6 +534,29 @@ static struct session_data *session_find(const char *source,
 	return NULL;
 }
 
+static int session_connect(struct session_data *session,
+						struct callback_data *callback)
+{
+	int err;
+
+	if (session->obex) {
+		g_idle_add(connection_complete, callback);
+		err = 0;
+	} else if (session->channel > 0) {
+		session->io = rfcomm_connect(&session->src, &session->dst,
+							session->channel,
+							rfcomm_callback,
+							callback);
+		err = (session->io == NULL) ? -EINVAL : 0;
+	} else {
+		callback->sdp = service_connect(&session->src, &session->dst,
+						service_callback, callback);
+		err = (callback->sdp == NULL) ? -ENOMEM : 0;
+	}
+
+	return err;
+}
+
 struct session_data *session_create(const char *source,
 						const char *destination,
 						const char *service,
@@ -544,7 +567,6 @@ struct session_data *session_create(const char *source,
 {
 	struct session_data *session;
 	struct callback_data *callback;
-	int err;
 
 	if (destination == NULL)
 		return NULL;
@@ -613,22 +635,7 @@ proceed:
 	callback->func = function;
 	callback->data = user_data;
 
-	if (session->obex) {
-		g_idle_add(connection_complete, callback);
-		err = 0;
-	} else if (session->channel > 0) {
-		session->io = rfcomm_connect(&session->src, &session->dst,
-							session->channel,
-							rfcomm_callback,
-							callback);
-		err = (session->io == NULL) ? -EINVAL : 0;
-	} else {
-		callback->sdp = service_connect(&session->src, &session->dst,
-						service_callback, callback);
-		err = (callback->sdp == NULL) ? -ENOMEM : 0;
-	}
-
-	if (err < 0) {
+	if (session_connect(session, callback) < 0) {
 		session_unref(session);
 		g_free(callback);
 		return NULL;
-- 
1.7.4.1

[PATCH 3/6] Add handling of system D-Bus method calls

[Dmitriy Paliy]

Sending system D-Bus method calls (DefaultAdapter and RequestSession)
and handling of pending D-Bus calls is added to obex-client.
---
 client/session.c |  186 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 client/session.h |    2 +
 2 files changed, 187 insertions(+), 1 deletions(-)

diff --git a/client/session.c b/client/session.c
index 4016e35..75cd32e 100644
--- a/client/session.c
+++ b/client/session.c
@@ -56,6 +56,11 @@
 
 #define OBEX_IO_ERROR obex_io_error_quark()
 
+#define BT_BUS_NAME		"org.bluez"
+#define BT_PATH			"/"
+#define BT_ADAPTER_IFACE	"org.bluez.Adapter"
+#define BT_MANAGER_IFACE	"org.bluez.Manager"
+
 static guint64 counter = 0;
 
 static unsigned char pcsuite_uuid[] = { 0x00, 0x00, 0x50, 0x05, 0x00, 0x00,
@@ -87,6 +92,11 @@ struct agent_data {
 	struct pending_data *pending;
 };
 
+struct pending_req {
+	DBusPendingCall *call;
+	void *user_data;
+};
+
 static GSList *sessions = NULL;
 
 static void session_prepare_put(struct session_data *session, GError *err,
@@ -176,10 +186,44 @@ static void session_unregistered(struct session_data *session)
 	session->path = NULL;
 }
 
+struct pending_req *find_session_request(const struct session_data *session,
+				const DBusPendingCall *call)
+{
+	GSList *l;
+
+	for (l = session->pending_calls; l; l = l->next) {
+		struct pending_req *req = l->data;
+
+		if (req->call == call)
+			return req;
+	}
+
+	return NULL;
+}
+
+static void pending_req_finalize(struct pending_req *req)
+{
+	if (!dbus_pending_call_get_completed(req->call))
+		dbus_pending_call_cancel(req->call);
+
+	dbus_pending_call_unref(req->call);
+	g_free(req);
+}
+
 static void session_free(struct session_data *session)
 {
+	GSList *l = session->pending_calls;
+
 	DBG("%p", session);
 
+	while (l) {
+		struct pending_req *req = l->data;
+		l = l->next;
+
+		session->pending_calls = g_slist_remove(session->pending_calls, req);
+		pending_req_finalize(req);
+	}
+
 	if (session->agent)
 		agent_release(session);
 
@@ -206,6 +250,7 @@ static void session_free(struct session_data *session)
 
 	sessions = g_slist_remove(sessions, session);
 
+	g_free(session->adapter);
 	g_free(session->callback);
 	g_free(session->path);
 	g_free(session->service);
@@ -213,6 +258,50 @@ static void session_free(struct session_data *session)
 	g_free(session);
 }
 
+static struct pending_req *send_method_call(DBusConnection *connection,
+				const char *dest, const char *path,
+				const char *interface, const char *method,
+				DBusPendingCallNotifyFunction cb,
+				void *user_data, int type, ...)
+{
+	DBusMessage *msg;
+	DBusPendingCall *call;
+	va_list args;
+	struct pending_req *req;
+
+	msg = dbus_message_new_method_call(dest, path, interface, method);
+	if (!msg) {
+		error("Unable to allocate new D-Bus %s message", method);
+		return NULL;
+	}
+
+	va_start(args, type);
+
+	if (!dbus_message_append_args_valist(msg, type, args)) {
+		dbus_message_unref(msg);
+		va_end(args);
+		return NULL;
+	}
+
+	va_end(args);
+
+	if (!dbus_connection_send_with_reply(connection, msg, &call, -1)) {
+		error("Sending %s failed", method);
+		dbus_message_unref(msg);
+		return NULL;
+	}
+
+	dbus_pending_call_set_notify(call, cb, user_data, NULL);
+
+	req = g_new0(struct pending_req, 1);
+	req->call = call;
+	req->user_data = user_data;
+
+	dbus_message_unref(msg);
+
+	return req;
+}
+
 void session_unref(struct session_data *session)
 {
 	gboolean ret;
@@ -557,6 +646,93 @@ static int session_connect(struct session_data *session,
 	return err;
 }
 
+static void adapter_request_reply(DBusPendingCall *call, void *user_data)
+{
+	DBusError err;
+	DBusMessage *reply;
+	struct callback_data *callback = user_data;
+	struct session_data *session = callback->session;
+	struct pending_req *req = find_session_request(session, call);
+
+	reply = dbus_pending_call_steal_reply(call);
+
+	session->pending_calls = g_slist_remove(session->pending_calls, req);
+	pending_req_finalize(req);
+
+	dbus_error_init(&err);
+	if (dbus_set_error_from_message(&err, reply)) {
+		error("manager replied with an error: %s, %s",
+				err.name, err.message);
+		dbus_error_free(&err);
+
+		goto failed;
+	}
+
+	if (session_connect(session, callback) < 0)
+		goto failed;
+
+	goto proceed;
+
+failed:
+	session_unref(session);
+	g_free(callback);
+
+proceed:
+	dbus_message_unref(reply);
+}
+
+static void manager_reply(DBusPendingCall *call, void *user_data)
+{
+	DBusError err;
+	DBusMessage *reply;
+	char *adapter;
+	struct callback_data *callback = user_data;
+	struct session_data *session = callback->session;
+	struct pending_req *req = find_session_request(session, call);
+
+	reply = dbus_pending_call_steal_reply(call);
+
+	session->pending_calls = g_slist_remove(session->pending_calls, req);
+	pending_req_finalize(req);
+
+	dbus_error_init(&err);
+	if (dbus_set_error_from_message(&err, reply)) {
+		error("manager replied with an error: %s, %s",
+				err.name, err.message);
+		dbus_error_free(&err);
+
+		goto failed;
+	}
+
+	if (dbus_message_get_args(reply, NULL,
+				DBUS_TYPE_OBJECT_PATH, &adapter,
+				DBUS_TYPE_INVALID)) {
+		DBG("adapter path %s", adapter);
+
+		session->adapter = g_strdup(adapter);
+		req = send_method_call(session->conn_system,
+					BT_BUS_NAME, adapter,
+					BT_ADAPTER_IFACE, "RequestSession",
+					adapter_request_reply, callback,
+					DBUS_TYPE_INVALID);
+		if (!req)
+			goto failed;
+
+		session->pending_calls = g_slist_prepend(session->pending_calls,
+									req);
+	} else
+		goto failed;
+
+	goto proceed;
+
+failed:
+	session_unref(session);
+	g_free(callback);
+
+proceed:
+	dbus_message_unref(reply);
+}
+
 struct session_data *session_create(const char *source,
 						const char *destination,
 						const char *service,
@@ -567,6 +743,7 @@ struct session_data *session_create(const char *source,
 {
 	struct session_data *session;
 	struct callback_data *callback;
+	struct pending_req *req;
 
 	if (destination == NULL)
 		return NULL;
@@ -635,12 +812,19 @@ proceed:
 	callback->func = function;
 	callback->data = user_data;
 
-	if (session_connect(session, callback) < 0) {
+	req = send_method_call(session->conn_system,
+				BT_BUS_NAME, BT_PATH,
+				BT_MANAGER_IFACE, "DefaultAdapter",
+				manager_reply, callback,
+				DBUS_TYPE_INVALID);
+	if (!req) {
 		session_unref(session);
 		g_free(callback);
 		return NULL;
 	}
 
+	session->pending_calls = g_slist_prepend(session->pending_calls, req);
+
 	if (owner)
 		session_set_owner(session, owner, owner_disconnected);
 
diff --git a/client/session.h b/client/session.h
index 554b494..39f5742 100644
--- a/client/session.h
+++ b/client/session.h
@@ -51,7 +51,9 @@ struct session_data {
 	gchar *owner;		/* Session owner */
 	guint watch;
 	GSList *pending;
+	GSList *pending_calls;
 	void *priv;
+	char *adapter;
 };
 
 typedef void (*session_callback_t) (struct session_data *session,
-- 
1.7.4.1

[PATCH 4/6] Add adapter ReleaseSession to obex-client

[Dmitriy Paliy]

Release of adapter session is added to obex-client upon closing OBEX
transfer.
---
 client/session.c |   38 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/client/session.c b/client/session.c
index 75cd32e..423eb8c 100644
--- a/client/session.c
+++ b/client/session.c
@@ -302,9 +302,34 @@ static struct pending_req *send_method_call(DBusConnection *connection,
 	return req;
 }
 
+static void adapter_release_reply(DBusPendingCall *call, void *user_data)
+{
+	DBusError err;
+	DBusMessage *reply;
+	struct session_data *session = user_data;
+	struct pending_req *req = find_session_request(session, call);
+
+	reply = dbus_pending_call_steal_reply(call);
+
+	dbus_error_init(&err);
+	if (dbus_set_error_from_message(&err, reply)) {
+		error("manager replied with an error: %s, %s",
+				err.name, err.message);
+		dbus_error_free(&err);
+	}
+
+	dbus_message_unref(reply);
+
+	session->pending_calls = g_slist_remove(session->pending_calls, req);
+	pending_req_finalize(req);
+
+	session_free(session);
+}
+
 void session_unref(struct session_data *session)
 {
 	gboolean ret;
+	struct pending_req *req;
 
 	ret = g_atomic_int_dec_and_test(&session->refcount);
 
@@ -313,7 +338,18 @@ void session_unref(struct session_data *session)
 	if (ret == FALSE)
 		return;
 
-	session_free(session);
+	req = send_method_call(session->conn_system,
+				BT_BUS_NAME, session->adapter,
+				BT_ADAPTER_IFACE, "ReleaseSession",
+				adapter_release_reply, session,
+				DBUS_TYPE_INVALID);
+	if (!req) {
+		g_free(session->adapter);
+		session_free(session);
+		return;
+	}
+
+	session->pending_calls = g_slist_prepend(session->pending_calls, req);
 }
 
 static void rfcomm_callback(GIOChannel *io, GError *err, gpointer user_data)
-- 
1.7.4.1

[PATCH 5/6] Add FindAdapter method call to obex-client

[Dmitriy Paliy]

---
 client/session.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/client/session.c b/client/session.c
index 423eb8c..d8e3bf2 100644
--- a/client/session.c
+++ b/client/session.c
@@ -848,11 +848,21 @@ proceed:
 	callback->func = function;
 	callback->data = user_data;
 
-	req = send_method_call(session->conn_system,
+	if (source) {
+		req = send_method_call(session->conn_system,
+				BT_BUS_NAME, BT_PATH,
+				BT_MANAGER_IFACE, "FindAdapter",
+				manager_reply, callback,
+				DBUS_TYPE_STRING, &source,
+				DBUS_TYPE_INVALID);
+	} else {
+		req = send_method_call(session->conn_system,
 				BT_BUS_NAME, BT_PATH,
 				BT_MANAGER_IFACE, "DefaultAdapter",
 				manager_reply, callback,
 				DBUS_TYPE_INVALID);
+	}
+
 	if (!req) {
 		session_unref(session);
 		g_free(callback);
-- 
1.7.4.1

[PATCH 6/6] Code clean-up: remove unnecessary brackets

[Dmitriy Paliy]

---
 client/session.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/client/session.c b/client/session.c
index d8e3bf2..ae7e213 100644
--- a/client/session.c
+++ b/client/session.c
@@ -241,9 +241,8 @@ static void session_free(struct session_data *session)
 	if (session->path)
 		session_unregistered(session);
 
-	if (session->conn) {
+	if (session->conn)
 		dbus_connection_unref(session->conn);
-	}
 
 	if (session->conn_system)
 		dbus_connection_unref(session->conn_system);
-- 
1.7.4.1

Re: [PATCH 4/6] Add adapter ReleaseSession to obex-client

[Luiz Augusto von Dentz]

Hi Dmitriy,

On Tue, Jun 28, 2011 at 12:39 AM, Dmitriy Paliy <dmitriy.paliy@gmail.com> wrote:
> Release of adapter session is added to obex-client upon closing OBEX
> transfer.
> ---
>  client/session.c |   38 +++++++++++++++++++++++++++++++++++++-
>  1 files changed, 37 insertions(+), 1 deletions(-)
>
> diff --git a/client/session.c b/client/session.c
> index 75cd32e..423eb8c 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -302,9 +302,34 @@ static struct pending_req *send_method_call(DBusConnection *connection,
>        return req;
>  }
>
> +static void adapter_release_reply(DBusPendingCall *call, void *user_data)
> +{
> +       DBusError err;
> +       DBusMessage *reply;
> +       struct session_data *session = user_data;
> +       struct pending_req *req = find_session_request(session, call);
> +
> +       reply = dbus_pending_call_steal_reply(call);
> +
> +       dbus_error_init(&err);
> +       if (dbus_set_error_from_message(&err, reply)) {
> +               error("manager replied with an error: %s, %s",
> +                               err.name, err.message);
> +               dbus_error_free(&err);
> +       }
> +
> +       dbus_message_unref(reply);
> +
> +       session->pending_calls = g_slist_remove(session->pending_calls, req);
> +       pending_req_finalize(req);
> +
> +       session_free(session);
> +}
> +
>  void session_unref(struct session_data *session)
>  {
>        gboolean ret;
> +       struct pending_req *req;
>
>        ret = g_atomic_int_dec_and_test(&session->refcount);
>
> @@ -313,7 +338,18 @@ void session_unref(struct session_data *session)
>        if (ret == FALSE)
>                return;
>
> -       session_free(session);
> +       req = send_method_call(session->conn_system,
> +                               BT_BUS_NAME, session->adapter,
> +                               BT_ADAPTER_IFACE, "ReleaseSession",
> +                               adapter_release_reply, session,
> +                               DBUS_TYPE_INVALID);
> +       if (!req) {
> +               g_free(session->adapter);
> +               session_free(session);
> +               return;
> +       }
> +
> +       session->pending_calls = g_slist_prepend(session->pending_calls, req);
>  }
>
>  static void rfcomm_callback(GIOChannel *io, GError *err, gpointer user_data)
> --
> 1.7.4.1
>
>

I don't think this is really necessary since you basically gonna
disconnect from system bus on session_free, so calling ReleaseSession
and wait for the reply will only add complexity and it also leaves the
session with 0 refcount which I don't think is a good idea. In the
other hand we may want to stay connected to the system bus so that we
don't have to reconnect every time when creating a new session, in
that case you would need to call ReleaseSession but I wouldn't bother
waiting for the reply since ReleaseSession only fails in case the
process doesn't own any session which is still fine since it doesn't
leave anything behind.

-- 
Luiz Augusto von Dentz

Re: [PATCH 5/6] Add FindAdapter method call to obex-client

[Luiz Augusto von Dentz]

Hi Dmitriy,

On Tue, Jun 28, 2011 at 12:39 AM, Dmitriy Paliy <dmitriy.paliy@gmail.com> wrote:
> ---
>  client/session.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/client/session.c b/client/session.c
> index 423eb8c..d8e3bf2 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -848,11 +848,21 @@ proceed:
>        callback->func = function;
>        callback->data = user_data;
>
> -       req = send_method_call(session->conn_system,
> +       if (source) {
> +               req = send_method_call(session->conn_system,
> +                               BT_BUS_NAME, BT_PATH,
> +                               BT_MANAGER_IFACE, "FindAdapter",
> +                               manager_reply, callback,
> +                               DBUS_TYPE_STRING, &source,
> +                               DBUS_TYPE_INVALID);
> +       } else {
> +               req = send_method_call(session->conn_system,
>                                BT_BUS_NAME, BT_PATH,
>                                BT_MANAGER_IFACE, "DefaultAdapter",
>                                manager_reply, callback,
>                                DBUS_TYPE_INVALID);
> +       }
> +
>        if (!req) {
>                session_unref(session);
>                g_free(callback);
> --
> 1.7.4.1

I guess this one belong to the same patch where DefaultAdapter call
was introduced since it actually fix requesting the session to the
wrong adapter in case it the address is not from the default.


-- 
Luiz Augusto von Dentz

Re: [PATCH 6/6] Code clean-up: remove unnecessary brackets

[Luiz Augusto von Dentz]

Hi,

On Tue, Jun 28, 2011 at 12:39 AM, Dmitriy Paliy <dmitriy.paliy@gmail.com> wrote:
> ---
>  client/session.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/client/session.c b/client/session.c
> index d8e3bf2..ae7e213 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -241,9 +241,8 @@ static void session_free(struct session_data *session)
>        if (session->path)
>                session_unregistered(session);
>
> -       if (session->conn) {
> +       if (session->conn)
>                dbus_connection_unref(session->conn);
> -       }
>
>        if (session->conn_system)
>                dbus_connection_unref(session->conn_system);
> --
> 1.7.4.1
>
>

This patch is fine, but I would like to suggest that we only include
in the series patches that depend on each other, this one we could
quickly apply without looking into the others. Btw, we started using
project names on the subject prefix to help identifying what project
the patch is for, you can do that by editing the .git/config where you
cloned the repository adding the following:

[format]
	subjectprefix = PATCH obexd


-- 
Luiz Augusto von Dentz