[PATCH obexd v1 0/6] client: rethink transfer data access in session API

[PATCH obexd v1 0/6] client: rethink transfer data access in session API

[Mikel Astiz]

This second version modifies patch v0 2/6 (now v1 3/6 due to new dependency) in order to use one single callback type in session API, according to the proposal from Luiz.

>From previous cover letter:

This patch series proposes a change in the session API such that the concept of "active transfer" (session->p) is removed from the API. This is possible once the callbacks provide the pointer to the transfer object, which can be used by the modules to access the data they are interested in.

This transfer object pointer is guaranteed to be valid during the duration of the callback, but nothing else can be assumed. In particular there is no ownership change involved.

The new approach is less error-prone and avoids API duplication between transfer and session APIs.

Mikel Astiz (6):
  client: Minor buffer access API changes
  client: Avoid GObex dependency from transfer.h
  client: Give transfer pointer in session callbacks
  client: Use new session callback style in modules
  client: Remove deprecated part of session API
  client: Remove transfer from queue before callback

 client/driver.c   |    2 +
 client/ftp.c      |   15 +++++---
 client/manager.c  |   14 ++++++--
 client/map.c      |   13 ++++---
 client/opp.c      |    3 ++
 client/pbap.c     |   27 +++++++++------
 client/session.c  |   93 ++++++++++++++--------------------------------------
 client/session.h  |    4 +--
 client/sync.c     |    4 ++-
 client/transfer.c |   14 ++++---
 client/transfer.h |    8 ++--
 11 files changed, 89 insertions(+), 108 deletions(-)

-- 
1.7.7.6

[PATCH obexd v1 1/6] client: Minor buffer access API changes

[Mikel Astiz]

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

Trivial changes in buffer getters in both session and transfer,
regarding the access of transfer parameters:
	- const qualifiers added, to avoid unwanted frees
	- Buffers are now returned as void* instead of guint8*
---
 client/pbap.c     |    2 +-
 client/session.c  |   17 +++++++----------
 client/session.h  |    2 +-
 client/transfer.c |   12 +++++++-----
 client/transfer.h |    6 +++---
 5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/client/pbap.c b/client/pbap.c
index baf2ca6..d96b651 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -294,7 +294,7 @@ static void pbap_setpath_cb(struct obc_session *session, GError *err,
 static void read_return_apparam(struct obc_session *session,
 				guint16 *phone_book_size, guint8 *new_missed_calls)
 {
-	struct apparam_hdr *hdr;
+	const struct apparam_hdr *hdr;
 	size_t size;
 
 	*phone_book_size = 0;
diff --git a/client/session.c b/client/session.c
index e277fa0..824ef49 100644
--- a/client/session.c
+++ b/client/session.c
@@ -1156,22 +1156,19 @@ int obc_session_get_contents(struct obc_session *session, char **contents,
 	return obc_transfer_get_contents(transfer, contents, size);
 }
 
-void *obc_session_get_params(struct obc_session *session, size_t *size)
+const void *obc_session_get_params(struct obc_session *session, size_t *size)
 {
 	struct obc_transfer *transfer;
-	struct obc_transfer_params params;
 
-	transfer= obc_session_get_transfer(session);
-	if (transfer == NULL)
-		return NULL;
+	transfer = obc_session_get_transfer(session);
+	if (transfer == NULL) {
+		if (size != NULL)
+			*size = 0;
 
-	if (obc_transfer_get_params(transfer, &params) < 0)
 		return NULL;
+	}
 
-	if (size)
-		*size = params.size;
-
-	return params.data;
+	return obc_transfer_get_params(transfer, size);
 }
 
 static void setpath_complete(struct obc_session *session, GError *err,
diff --git a/client/session.h b/client/session.h
index c443392..b44cf3f 100644
--- a/client/session.h
+++ b/client/session.h
@@ -54,7 +54,7 @@ const char *obc_session_get_path(struct obc_session *session);
 const char *obc_session_get_target(struct obc_session *session);
 int obc_session_get_contents(struct obc_session *session, char **contents,
 								size_t *size);
-void *obc_session_get_params(struct obc_session *session, size_t *size);
+const void *obc_session_get_params(struct obc_session *session, size_t *size);
 
 guint obc_session_send(struct obc_session *session, const char *filename,
 				const char *name, GError **err);
diff --git a/client/transfer.c b/client/transfer.c
index 8b5d126..89690d0 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -631,13 +631,15 @@ guint8 obc_transfer_get_operation(struct obc_transfer *transfer)
 	return transfer->op;
 }
 
-int obc_transfer_get_params(struct obc_transfer *transfer,
-					struct obc_transfer_params *params)
+const void *obc_transfer_get_params(struct obc_transfer *transfer, size_t *size)
 {
-	params->data = transfer->params->data;
-	params->size = transfer->params->size;
+	if (transfer->params == NULL)
+		return NULL;
 
-	return 0;
+	if (size != NULL)
+		*size = transfer->params->size;
+
+	return transfer->params->data;
 }
 
 int obc_transfer_get_contents(struct obc_transfer *transfer, char **contents,
diff --git a/client/transfer.h b/client/transfer.h
index 3f5e22d..f42e21f 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -22,7 +22,7 @@
  */
 
 struct obc_transfer_params {
-	guint8 *data;
+	void *data;
 	size_t size;
 };
 
@@ -59,8 +59,8 @@ gboolean obc_transfer_start(struct obc_transfer *transfer, GObex *obex,
 								GError **err);
 guint8 obc_transfer_get_operation(struct obc_transfer *transfer);
 
-int obc_transfer_get_params(struct obc_transfer *transfer,
-					struct obc_transfer_params *params);
+const void *obc_transfer_get_params(struct obc_transfer *transfer,
+								size_t *size);
 int obc_transfer_get_contents(struct obc_transfer *transfer, char **contents,
 								size_t *size);
 
-- 
1.7.7.6

[PATCH obexd v1 2/6] client: Avoid GObex dependency from transfer.h

[Mikel Astiz]

This workaround makes it possible to include transfer.h from the
modules, without adding a dependency to GObex.
---
 client/transfer.c |    2 +-
 client/transfer.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index 89690d0..411a7c0 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -610,7 +610,7 @@ static gboolean transfer_start_put(struct obc_transfer *transfer, GError **err)
 	return TRUE;
 }
 
-gboolean obc_transfer_start(struct obc_transfer *transfer, GObex *obex,
+gboolean obc_transfer_start(struct obc_transfer *transfer, void *obex,
 								GError **err)
 {
 	transfer->obex = g_obex_ref(obex);
diff --git a/client/transfer.h b/client/transfer.h
index f42e21f..1b83d18 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -55,7 +55,7 @@ gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
 					transfer_callback_t func,
 					void *user_data);
 
-gboolean obc_transfer_start(struct obc_transfer *transfer, GObex *obex,
+gboolean obc_transfer_start(struct obc_transfer *transfer, void *obex,
 								GError **err);
 guint8 obc_transfer_get_operation(struct obc_transfer *transfer);
 
-- 
1.7.7.6

[PATCH obexd v1 3/6] client: Give transfer pointer in session callbacks

[Mikel Astiz]

Operations involving a transfer object will receive a pointer to such
transfer in the callback.

Note that the ownership of this object is not changed in any way,
meaning that the session is still responsible for it. However this
pointer can be useful during the execution of the callback, in order to
access data members of the transfer.
---
 client/driver.c  |    2 ++
 client/ftp.c     |   13 ++++++++-----
 client/manager.c |   12 +++++++++---
 client/map.c     |   11 +++++++----
 client/opp.c     |    3 +++
 client/pbap.c    |   15 ++++++++++-----
 client/session.c |   34 ++++++++++++++++++----------------
 client/session.h |    1 +
 client/sync.c    |    2 ++
 9 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/client/driver.c b/client/driver.c
index f9e8fbc..fe61219 100644
--- a/client/driver.c
+++ b/client/driver.c
@@ -28,7 +28,9 @@
 #include <string.h>
 #include <errno.h>
 #include <glib.h>
+#include <gdbus.h>
 
+#include "transfer.h"
 #include "session.h"
 #include "driver.h"
 #include "log.h"
diff --git a/client/ftp.c b/client/ftp.c
index 0e6af47..0cb3adc 100644
--- a/client/ftp.c
+++ b/client/ftp.c
@@ -32,6 +32,7 @@
 
 #include "log.h"
 
+#include "transfer.h"
 #include "session.h"
 #include "driver.h"
 #include "ftp.h"
@@ -50,8 +51,8 @@ struct ftp_data {
 	struct obc_session *session;
 };
 
-static void async_cb(struct obc_session *session, GError *err,
-							gpointer user_data)
+static void async_cb(struct obc_session *session, struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	DBusMessage *reply, *msg = user_data;
 
@@ -172,8 +173,9 @@ static const GMarkupParser parser = {
 	NULL
 };
 
-static void get_file_callback(struct obc_session *session, GError *err,
-							void *user_data)
+static void get_file_callback(struct obc_session *session,
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	DBusMessage *msg = user_data;
 	DBusMessage *reply;
@@ -190,7 +192,8 @@ static void get_file_callback(struct obc_session *session, GError *err,
 }
 
 static void list_folder_callback(struct obc_session *session,
-					GError *err, void *user_data)
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	DBusMessage *msg = user_data;
 	GMarkupParseContext *ctxt;
diff --git a/client/manager.c b/client/manager.c
index 6d08702..820ef37 100644
--- a/client/manager.c
+++ b/client/manager.c
@@ -35,6 +35,7 @@
 #include <gdbus.h>
 
 #include "log.h"
+#include "transfer.h"
 #include "session.h"
 #include "manager.h"
 #include "bluetooth.h"
@@ -78,8 +79,9 @@ static void unregister_session(void *data)
 	obc_session_unref(session);
 }
 
-static void create_callback(struct obc_session *session, GError *err,
-							void *user_data)
+static void create_callback(struct obc_session *session,
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	struct send_data *data = user_data;
 	unsigned int i;
@@ -247,6 +249,7 @@ static DBusMessage *send_files(DBusConnection *connection,
 }
 
 static void pull_complete_callback(struct obc_session *session,
+					struct obc_transfer *transfer,
 					GError *err, void *user_data)
 {
 	struct send_data *data = user_data;
@@ -271,7 +274,8 @@ done:
 }
 
 static void pull_obc_session_callback(struct obc_session *session,
-					GError *err, void *user_data)
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	struct send_data *data = user_data;
 	DBusMessage *reply;
@@ -448,6 +452,7 @@ static DBusMessage *remove_session(DBusConnection *connection,
 }
 
 static void capabilities_complete_callback(struct obc_session *session,
+						struct obc_transfer *transfer,
 						GError *err, void *user_data)
 {
 	struct send_data *data = user_data;
@@ -488,6 +493,7 @@ done:
 }
 
 static void capability_obc_session_callback(struct obc_session *session,
+						struct obc_transfer *transfer,
 						GError *err, void *user_data)
 {
 	struct send_data *data = user_data;
diff --git a/client/map.c b/client/map.c
index 1b4e404..50d02c6 100644
--- a/client/map.c
+++ b/client/map.c
@@ -32,6 +32,7 @@
 #include "log.h"
 
 #include "map.h"
+#include "transfer.h"
 #include "session.h"
 #include "driver.h"
 
@@ -49,8 +50,9 @@ struct map_data {
 
 static DBusConnection *conn = NULL;
 
-static void simple_cb(struct obc_session *session, GError *err,
-							gpointer user_data)
+static void simple_cb(struct obc_session *session,
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	DBusMessage *reply;
 	struct map_data *map = user_data;
@@ -94,8 +96,9 @@ static DBusMessage *map_setpath(DBusConnection *connection,
 	return NULL;
 }
 
-static void buffer_cb(struct obc_session *session, GError *err,
-							void *user_data)
+static void buffer_cb(struct obc_session *session,
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	struct map_data *map = user_data;
 	DBusMessage *reply;
diff --git a/client/opp.c b/client/opp.c
index be382ef..efbf3e9 100644
--- a/client/opp.c
+++ b/client/opp.c
@@ -25,8 +25,11 @@
 #include <config.h>
 #endif
 
+#include <gdbus.h>
+
 #include "log.h"
 
+#include "transfer.h"
 #include "session.h"
 #include "driver.h"
 #include "opp.h"
diff --git a/client/pbap.c b/client/pbap.c
index d96b651..c43d8dd 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -36,6 +36,7 @@
 
 #include "log.h"
 
+#include "transfer.h"
 #include "session.h"
 #include "driver.h"
 #include "pbap.h"
@@ -271,8 +272,9 @@ static void pbap_reset_path(struct pbap_data *pbap)
 	obc_session_setpath(pbap->session, pbap->path, NULL, NULL, NULL);
 }
 
-static void pbap_setpath_cb(struct obc_session *session, GError *err,
-							gpointer user_data)
+static void pbap_setpath_cb(struct obc_session *session,
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	struct pending_request *request = user_data;
 	struct pbap_data *pbap = request->pbap;
@@ -339,7 +341,8 @@ static void read_return_apparam(struct obc_session *session,
 }
 
 static void pull_phonebook_callback(struct obc_session *session,
-					GError *err, void *user_data)
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	struct pending_request *request = user_data;
 	DBusMessage *reply;
@@ -377,7 +380,8 @@ send:
 }
 
 static void phonebook_size_callback(struct obc_session *session,
-					GError *err, void *user_data)
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	struct pending_request *request = user_data;
 	DBusMessage *reply;
@@ -405,7 +409,8 @@ send:
 }
 
 static void pull_vcard_listing_callback(struct obc_session *session,
-					GError *err, void *user_data)
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	struct pending_request *request = user_data;
 	GMarkupParseContext *ctxt;
diff --git a/client/session.c b/client/session.c
index 824ef49..408428c 100644
--- a/client/session.c
+++ b/client/session.c
@@ -245,7 +245,7 @@ static void connect_cb(GObex *obex, GError *err, GObexPacket *rsp,
 				"OBEX Connect failed with 0x%02x", rsp_code);
 
 done:
-	callback->func(callback->session, gerr, callback->data);
+	callback->func(callback->session, NULL, gerr, callback->data);
 	if (gerr != NULL)
 		g_error_free(gerr);
 	obc_session_unref(callback->session);
@@ -303,7 +303,7 @@ static void transport_func(GIOChannel *io, GError *err, gpointer user_data)
 
 	return;
 done:
-	callback->func(callback->session, err, callback->data);
+	callback->func(callback->session, NULL, err, callback->data);
 	obc_session_unref(callback->session);
 	g_free(callback);
 }
@@ -373,7 +373,7 @@ static gboolean connection_complete(gpointer data)
 {
 	struct callback_data *cb = data;
 
-	cb->func(cb->session, 0, cb->data);
+	cb->func(cb->session, NULL, NULL, cb->data);
 
 	obc_session_unref(cb->session);
 
@@ -498,7 +498,8 @@ void obc_session_shutdown(struct obc_session *session)
 
 	if (session->p != NULL && session->p->id != 0) {
 		if (session->p->func)
-			session->p->func(session, err, session->p->data);
+			session->p->func(session, session->p->transfer, err,
+							session->p->data);
 
 		pending_request_free(session->p);
 		session->p = NULL;
@@ -506,7 +507,7 @@ void obc_session_shutdown(struct obc_session *session)
 
 	while ((p = g_queue_pop_head(session->queue))) {
 		if (p->func)
-			p->func(session, err, p->data);
+			p->func(session, p->transfer, err, p->data);
 
 		pending_request_free(p);
 	}
@@ -798,7 +799,7 @@ static void session_process_queue(struct obc_session *session)
 
 			g_set_error(&gerr, OBEX_IO_ERROR, err,
 							"Authorization failed");
-			p->func(session, gerr, p->data);
+			p->func(session, p->transfer, gerr, p->data);
 			g_error_free(gerr);
 		}
 
@@ -842,7 +843,7 @@ static void session_terminate_transfer(struct obc_session *session,
 	obc_session_ref(session);
 
 	if (p->func)
-		p->func(session, gerr, p->data);
+		p->func(session, p->transfer, gerr, p->data);
 
 	pending_request_free(p);
 
@@ -1171,14 +1172,15 @@ const void *obc_session_get_params(struct obc_session *session, size_t *size)
 	return obc_transfer_get_params(transfer, size);
 }
 
-static void setpath_complete(struct obc_session *session, GError *err,
-							void *user_data)
+static void setpath_complete(struct obc_session *session,
+						struct obc_transfer *transfer,
+						GError *err, void *user_data)
 {
 	struct pending_request *p = user_data;
 	struct setpath_data *data = p->data;
 
 	if (data->func)
-		data->func(session, err, data->user_data);
+		data->func(session, NULL, err, data->user_data);
 
 	g_strfreev(data->remaining);
 	g_free(data);
@@ -1202,7 +1204,7 @@ static void setpath_cb(GObex *obex, GError *err, GObexPacket *rsp,
 	p->req_id = 0;
 
 	if (err != NULL) {
-		setpath_complete(p->session, err, user_data);
+		setpath_complete(p->session, NULL, err, user_data);
 		return;
 	}
 
@@ -1211,14 +1213,14 @@ static void setpath_cb(GObex *obex, GError *err, GObexPacket *rsp,
 		GError *gerr = NULL;
 		g_set_error(&gerr, OBEX_IO_ERROR, code, "%s",
 							g_obex_strerror(code));
-		setpath_complete(p->session, err, user_data);
+		setpath_complete(p->session, NULL, err, user_data);
 		g_clear_error(&gerr);
 		return;
 	}
 
 	next = data->remaining[data->index];
 	if (next == NULL) {
-		setpath_complete(p->session, NULL, user_data);
+		setpath_complete(p->session, NULL, NULL, user_data);
 		return;
 	}
 
@@ -1226,7 +1228,7 @@ static void setpath_cb(GObex *obex, GError *err, GObexPacket *rsp,
 
 	p->req_id = g_obex_setpath(obex, next, setpath_cb, p, &err);
 	if (err != NULL) {
-		setpath_complete(p->session, err, data);
+		setpath_complete(p->session, NULL, err, data);
 		g_error_free(err);
 	}
 }
@@ -1291,7 +1293,7 @@ static void async_cb(GObex *obex, GError *err, GObexPacket *rsp,
 
 	if (err != NULL) {
 		if (p->func)
-			p->func(p->session, err, p->data);
+			p->func(p->session, NULL, err, p->data);
 		goto done;
 	}
 
@@ -1301,7 +1303,7 @@ static void async_cb(GObex *obex, GError *err, GObexPacket *rsp,
 							g_obex_strerror(code));
 
 	if (p->func)
-		p->func(p->session, gerr, p->data);
+		p->func(p->session, NULL, gerr, p->data);
 
 	if (gerr != NULL)
 		g_clear_error(&gerr);
diff --git a/client/session.h b/client/session.h
index b44cf3f..ca97900 100644
--- a/client/session.h
+++ b/client/session.h
@@ -28,6 +28,7 @@
 struct obc_session;
 
 typedef void (*session_callback_t) (struct obc_session *session,
+					struct obc_transfer *transfer,
 					GError *err, void *user_data);
 
 struct obc_session *obc_session_create(const char *source,
diff --git a/client/sync.c b/client/sync.c
index 9a26f5b..c549040 100644
--- a/client/sync.c
+++ b/client/sync.c
@@ -34,6 +34,7 @@
 
 #include "log.h"
 
+#include "transfer.h"
 #include "session.h"
 #include "driver.h"
 #include "sync.h"
@@ -84,6 +85,7 @@ static DBusMessage *sync_setlocation(DBusConnection *connection,
 }
 
 static void sync_getphonebook_callback(struct obc_session *session,
+					struct obc_transfer *transfer,
 					GError *err, void *user_data)
 {
 	struct sync_data *sync = user_data;
-- 
1.7.7.6

[PATCH obexd v1 4/6] client: Use new session callback style in modules

[Mikel Astiz]

The session API now provides the transfer object in the callback, so
the modules can directly access the transfer object.
---
 client/ftp.c     |    2 +-
 client/manager.c |    2 +-
 client/map.c     |    2 +-
 client/pbap.c    |   10 +++++-----
 client/sync.c    |    2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/client/ftp.c b/client/ftp.c
index 0cb3adc..86c2a3c 100644
--- a/client/ftp.c
+++ b/client/ftp.c
@@ -204,7 +204,7 @@ static void list_folder_callback(struct obc_session *session,
 
 	reply = dbus_message_new_method_return(msg);
 
-	if (obc_session_get_contents(session, &contents, &size) < 0)
+	if (obc_transfer_get_contents(transfer, &contents, &size) < 0)
 		goto done;
 
 	dbus_message_iter_init_append(reply, &iter);
diff --git a/client/manager.c b/client/manager.c
index 820ef37..a46519f 100644
--- a/client/manager.c
+++ b/client/manager.c
@@ -468,7 +468,7 @@ static void capabilities_complete_callback(struct obc_session *session,
 		goto done;
 	}
 
-	perr = obc_session_get_contents(session, &contents, &size);
+	perr = obc_transfer_get_contents(transfer, &contents, &size);
 	if (perr < 0) {
 		DBusMessage *error = g_dbus_create_error(data->message,
 						"org.openobex.Error.Failed",
diff --git a/client/map.c b/client/map.c
index 50d02c6..eb06834 100644
--- a/client/map.c
+++ b/client/map.c
@@ -113,7 +113,7 @@ static void buffer_cb(struct obc_session *session,
 		goto done;
 	}
 
-	perr = obc_session_get_contents(session, &contents, &size);
+	perr = obc_transfer_get_contents(transfer, &contents, &size);
 	if (perr < 0) {
 		reply = g_dbus_create_error(map->msg,
 						"org.openobex.Error.Failed",
diff --git a/client/pbap.c b/client/pbap.c
index c43d8dd..b178aff 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -293,7 +293,7 @@ static void pbap_setpath_cb(struct obc_session *session,
 	pending_request_free(request);
 }
 
-static void read_return_apparam(struct obc_session *session,
+static void read_return_apparam(struct obc_transfer *transfer,
 				guint16 *phone_book_size, guint8 *new_missed_calls)
 {
 	const struct apparam_hdr *hdr;
@@ -302,7 +302,7 @@ static void read_return_apparam(struct obc_session *session,
 	*phone_book_size = 0;
 	*new_missed_calls = 0;
 
-	hdr = obc_session_get_params(session, &size);
+	hdr = obc_transfer_get_params(transfer, &size);
 	if (hdr == NULL)
 		return;
 
@@ -357,7 +357,7 @@ static void pull_phonebook_callback(struct obc_session *session,
 		goto send;
 	}
 
-	perr = obc_session_get_contents(session, &contents, &size);
+	perr = obc_transfer_get_contents(transfer, &contents, &size);
 	if (perr < 0) {
 		reply = g_dbus_create_error(request->msg,
 						"org.openobex.Error.Failed",
@@ -397,7 +397,7 @@ static void phonebook_size_callback(struct obc_session *session,
 
 	reply = dbus_message_new_method_return(request->msg);
 
-	read_return_apparam(session, &phone_book_size, &new_missed_calls);
+	read_return_apparam(transfer, &phone_book_size, &new_missed_calls);
 
 	dbus_message_append_args(reply,
 			DBUS_TYPE_UINT16, &phone_book_size,
@@ -427,7 +427,7 @@ static void pull_vcard_listing_callback(struct obc_session *session,
 		goto send;
 	}
 
-	perr = obc_session_get_contents(session, &contents, &size);
+	perr = obc_transfer_get_contents(transfer, &contents, &size);
 	if (perr < 0) {
 		reply = g_dbus_create_error(request->msg,
 						"org.openobex.Error.Failed",
diff --git a/client/sync.c b/client/sync.c
index c549040..c0b3800 100644
--- a/client/sync.c
+++ b/client/sync.c
@@ -101,7 +101,7 @@ static void sync_getphonebook_callback(struct obc_session *session,
 		goto send;
 	}
 
-	perr = obc_session_get_contents(session, &contents, &size);
+	perr = obc_transfer_get_contents(transfer, &contents, &size);
 	if (perr < 0) {
 		reply = g_dbus_create_error(sync->msg,
 						"org.openobex.Error.Failed",
-- 
1.7.7.6

[PATCH obexd v1 5/6] client: Remove deprecated part of session API

[Mikel Astiz]

Once the modules are using the new callback style, the session API can
be simplified and the old functions to access session->p removed.
---
 client/session.c |   40 ----------------------------------------
 client/session.h |    3 ---
 2 files changed, 0 insertions(+), 43 deletions(-)

diff --git a/client/session.c b/client/session.c
index 408428c..b994080 100644
--- a/client/session.c
+++ b/client/session.c
@@ -1132,46 +1132,6 @@ const char *obc_session_get_target(struct obc_session *session)
 	return session->driver->target;
 }
 
-static struct obc_transfer *obc_session_get_transfer(
-						struct obc_session *session)
-{
-	if (session->p == NULL)
-		return NULL;
-
-	return session->p->transfer;
-}
-
-int obc_session_get_contents(struct obc_session *session, char **contents,
-								size_t *size)
-{
-	struct obc_transfer *transfer;
-
-	transfer = obc_session_get_transfer(session);
-	if (transfer == NULL) {
-		if (size != NULL)
-			*size = 0;
-
-		return -EINVAL;
-	}
-
-	return obc_transfer_get_contents(transfer, contents, size);
-}
-
-const void *obc_session_get_params(struct obc_session *session, size_t *size)
-{
-	struct obc_transfer *transfer;
-
-	transfer = obc_session_get_transfer(session);
-	if (transfer == NULL) {
-		if (size != NULL)
-			*size = 0;
-
-		return NULL;
-	}
-
-	return obc_transfer_get_params(transfer, size);
-}
-
 static void setpath_complete(struct obc_session *session,
 						struct obc_transfer *transfer,
 						GError *err, void *user_data)
diff --git a/client/session.h b/client/session.h
index ca97900..06aaa1b 100644
--- a/client/session.h
+++ b/client/session.h
@@ -53,9 +53,6 @@ const char *obc_session_get_agent(struct obc_session *session);
 
 const char *obc_session_get_path(struct obc_session *session);
 const char *obc_session_get_target(struct obc_session *session);
-int obc_session_get_contents(struct obc_session *session, char **contents,
-								size_t *size);
-const void *obc_session_get_params(struct obc_session *session, size_t *size);
 
 guint obc_session_send(struct obc_session *session, const char *filename,
 				const char *name, GError **err);
-- 
1.7.7.6

[PATCH obexd v1 6/6] client: Remove transfer from queue before callback

[Mikel Astiz]

It is safer to remove the transfer from the internal queue (including
session->p) before calling the transfer callback. This makes sure the
callback will not manipulate the session in a way that the transfer is
removed more than once.

This was previously protected with session->p->id != 0 checks, but once
the new callbacks have been adopted in session API, this logic can be
removed.
---
 client/session.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/client/session.c b/client/session.c
index b994080..3a8807b 100644
--- a/client/session.c
+++ b/client/session.c
@@ -497,12 +497,13 @@ void obc_session_shutdown(struct obc_session *session)
 						"Session closed by user");
 
 	if (session->p != NULL && session->p->id != 0) {
-		if (session->p->func)
-			session->p->func(session, session->p->transfer, err,
-							session->p->data);
-
-		pending_request_free(session->p);
+		p = session->p;
 		session->p = NULL;
+
+		if (p->func)
+			p->func(session, p->transfer, err, p->data);
+
+		pending_request_free(p);
 	}
 
 	while ((p = g_queue_pop_head(session->queue))) {
@@ -836,9 +837,8 @@ static void session_terminate_transfer(struct obc_session *session,
 
 		p = match->data;
 		g_queue_delete_link(session->queue, match);
-	}
-
-	p->id = 0;
+	} else
+		session->p = NULL;
 
 	obc_session_ref(session);
 
@@ -847,10 +847,8 @@ static void session_terminate_transfer(struct obc_session *session,
 
 	pending_request_free(p);
 
-	if (p == session->p) {
-		session->p = NULL;
+	if (session->p == NULL)
 		session_process_queue(session);
-	}
 
 	obc_session_unref(session);
 }
-- 
1.7.7.6

Re: [PATCH obexd v1 0/6] client: rethink transfer data access in session API

[Luiz Augusto von Dentz]

Hi Mikel,

On Mon, Apr 30, 2012 at 6:26 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> This second version modifies patch v0 2/6 (now v1 3/6 due to new dependency) in order to use one single callback type in session API, according to the proposal from Luiz.
>
> From previous cover letter:
>
> This patch series proposes a change in the session API such that the concept of "active transfer" (session->p) is removed from the API. This is possible once the callbacks provide the pointer to the transfer object, which can be used by the modules to access the data they are interested in.
>
> This transfer object pointer is guaranteed to be valid during the duration of the callback, but nothing else can be assumed. In particular there is no ownership change involved.
>
> The new approach is less error-prone and avoids API duplication between transfer and session APIs.
>
> Mikel Astiz (6):
>  client: Minor buffer access API changes
>  client: Avoid GObex dependency from transfer.h
>  client: Give transfer pointer in session callbacks
>  client: Use new session callback style in modules
>  client: Remove deprecated part of session API
>  client: Remove transfer from queue before callback
>
>  client/driver.c   |    2 +
>  client/ftp.c      |   15 +++++---
>  client/manager.c  |   14 ++++++--
>  client/map.c      |   13 ++++---
>  client/opp.c      |    3 ++
>  client/pbap.c     |   27 +++++++++------
>  client/session.c  |   93 ++++++++++++++--------------------------------------
>  client/session.h  |    4 +--
>  client/sync.c     |    4 ++-
>  client/transfer.c |   14 ++++---
>  client/transfer.h |    8 ++--
>  11 files changed, 89 insertions(+), 108 deletions(-)
>
> --
> 1.7.7.6

I reviewed and did some testing but when I was about to apply them I
noticed that your email has changed after 1/6, could you please fix
that?


-- 
Luiz Augusto von Dentz