linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH obexd v2 0/6] Several minor fixes
@ 2012-02-14 14:47 Mikel Astiz
  2012-02-14 14:47 ` [PATCH obexd v2 1/6] client: fix memory leak in obc_session_put Mikel Astiz
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Mikel Astiz @ 2012-02-14 14:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

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

This second series includes the patches acked in the previous proposal (patches 1, 4, 5, 6) and the reviewed patches 2 and 3.

Patch 2 now considers the possibility of unreferencing the session inside the transfer callback.

Patch 3 now follows more strictly the glib convention on error reporting parameters.

Mikel Astiz (6):
  client: fix memory leak in obc_session_put
  client: fix unreported error case
  client: fix incorrect error check
  client: fix pbap select when same path given twice
  client: queue transfers in ftp sessions
  client: queue transfers in pbap sessions

 client/ftp.c     |   39 ++++---------
 client/pbap.c    |  176 ++++++++++++++++++++++++------------------------------
 client/session.c |   22 ++++++-
 3 files changed, 109 insertions(+), 128 deletions(-)

-- 
1.7.6.5


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

* [PATCH obexd v2 1/6] client: fix memory leak in obc_session_put
  2012-02-14 14:47 [PATCH obexd v2 0/6] Several minor fixes Mikel Astiz
@ 2012-02-14 14:47 ` Mikel Astiz
  2012-02-14 14:47 ` [PATCH obexd v2 2/6] client: fix unreported error case Mikel Astiz
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Mikel Astiz @ 2012-02-14 14:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

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

obc_session_put takes ownership of the given buffer, but did not free
the memory in case of error.
---
 client/session.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/client/session.c b/client/session.c
index 0fa8efc..28516b2 100644
--- a/client/session.c
+++ b/client/session.c
@@ -1017,8 +1017,10 @@ int obc_session_put(struct obc_session *session, char *buf, const char *targetna
 	struct obc_transfer *transfer;
 	const char *agent;
 
-	if (session->obex == NULL)
+	if (session->obex == NULL) {
+		g_free(buf);
 		return -ENOTCONN;
+	}
 
 	agent = obc_agent_get_name(session->agent);
 
@@ -1026,8 +1028,10 @@ int obc_session_put(struct obc_session *session, char *buf, const char *targetna
 							agent, NULL,
 							targetname, NULL,
 							NULL);
-	if (transfer == NULL)
+	if (transfer == NULL) {
+		g_free(buf);
 		return -EIO;
+	}
 
 	obc_transfer_set_buffer(transfer, buf);
 
-- 
1.7.6.5


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

* [PATCH obexd v2 2/6] client: fix unreported error case
  2012-02-14 14:47 [PATCH obexd v2 0/6] Several minor fixes Mikel Astiz
  2012-02-14 14:47 ` [PATCH obexd v2 1/6] client: fix memory leak in obc_session_put Mikel Astiz
@ 2012-02-14 14:47 ` Mikel Astiz
  2012-02-16 13:07   ` Johan Hedberg
  2012-02-14 14:47 ` [PATCH obexd v2 3/6] client: fix incorrect error check Mikel Astiz
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Mikel Astiz @ 2012-02-14 14:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

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

The authorization request of a queued transfer could fail, and this
needs to be reported to the transfer initiator. Otherwise it would
likely result in D-Bus timeouts.
---
 client/session.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/client/session.c b/client/session.c
index 28516b2..2f6df75 100644
--- a/client/session.c
+++ b/client/session.c
@@ -735,17 +735,31 @@ static void session_process_queue(struct obc_session *session)
 	if (session->queue == NULL || g_queue_is_empty(session->queue))
 		return;
 
+	obc_session_ref(session);
+
 	while ((p = g_queue_pop_head(session->queue))) {
 		int err;
 
 		err = pending_request_auth(p);
 		if (err == 0) {
 			session->p = p;
+			obc_session_unref(session);
 			return;
 		}
 
+		if (p->func) {
+			GError *gerr;
+
+			g_set_error(&gerr, OBEX_IO_ERROR, err,
+							"Authorization failed");
+			p->func(session, gerr, p->data);
+			g_error_free(gerr);
+		}
+
 		pending_request_free(p);
 	}
+
+	obc_session_unref(session);
 }
 
 static void session_terminate_transfer(struct obc_session *session,
-- 
1.7.6.5


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

* [PATCH obexd v2 3/6] client: fix incorrect error check
  2012-02-14 14:47 [PATCH obexd v2 0/6] Several minor fixes Mikel Astiz
  2012-02-14 14:47 ` [PATCH obexd v2 1/6] client: fix memory leak in obc_session_put Mikel Astiz
  2012-02-14 14:47 ` [PATCH obexd v2 2/6] client: fix unreported error case Mikel Astiz
@ 2012-02-14 14:47 ` Mikel Astiz
  2012-02-14 14:47 ` [PATCH obexd v2 4/6] client: fix pbap select when same path given twice Mikel Astiz
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Mikel Astiz @ 2012-02-14 14:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

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

Previous statement always returned success.
---
 client/pbap.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/pbap.c b/client/pbap.c
index 4910536..4bb7a70 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -300,8 +300,8 @@ static gboolean pbap_setpath(struct pbap_data *pbap, const char *location,
 		return TRUE;
 	}
 
-	obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap, err);
-	if (err != NULL) {
+	if (obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap,
+									err)) {
 		g_free(pbap->path);
 		pbap->path = path;
 		return TRUE;
-- 
1.7.6.5


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

* [PATCH obexd v2 4/6] client: fix pbap select when same path given twice
  2012-02-14 14:47 [PATCH obexd v2 0/6] Several minor fixes Mikel Astiz
                   ` (2 preceding siblings ...)
  2012-02-14 14:47 ` [PATCH obexd v2 3/6] client: fix incorrect error check Mikel Astiz
@ 2012-02-14 14:47 ` Mikel Astiz
  2012-02-14 14:47 ` [PATCH obexd v2 5/6] client: queue transfers in ftp sessions Mikel Astiz
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Mikel Astiz @ 2012-02-14 14:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

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

If the same path is selected twice, the operation can be skipped but the
D-Bus response should still be sent.
---
 client/pbap.c |   57 ++++++++++++++++-----------------------------------------
 1 files changed, 16 insertions(+), 41 deletions(-)

diff --git a/client/pbap.c b/client/pbap.c
index 4bb7a70..c4b14a2 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -124,12 +124,6 @@ static const char *filter_list[] = {
 #define PBAP_INTERFACE  "org.openobex.PhonebookAccess"
 #define PBAP_UUID "0000112f-0000-1000-8000-00805f9b34fb"
 
-#define PBAP_ERROR pbap_error_quark()
-
-enum {
-	PBAP_INVALID_PATH,
-};
-
 struct pbap_data {
 	struct obc_session *session;
 	char *path;
@@ -173,11 +167,6 @@ struct apparam_hdr {
 
 static DBusConnection *conn = NULL;
 
-static GQuark pbap_error_quark(void)
-{
-	return g_quark_from_static_string("pbap-error-quark");
-}
-
 static void listing_element(GMarkupParseContext *ctxt,
 				const gchar *element,
 				const gchar **names,
@@ -283,35 +272,6 @@ static void pbap_setpath_cb(struct obc_session *session, GError *err,
 	pbap->msg = NULL;
 }
 
-static gboolean pbap_setpath(struct pbap_data *pbap, const char *location,
-					const char *item, GError **err)
-{
-	char *path;
-
-	path = build_phonebook_path(location, item);
-	if (path == NULL) {
-		g_set_error(err, PBAP_ERROR, PBAP_INVALID_PATH,
-							"Invalid path");
-		return FALSE;
-	}
-
-	if (pbap->path != NULL && g_str_equal(pbap->path, path)) {
-		g_free(path);
-		return TRUE;
-	}
-
-	if (obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap,
-									err)) {
-		g_free(pbap->path);
-		pbap->path = path;
-		return TRUE;
-	}
-
-	g_free(path);
-
-	return FALSE;
-}
-
 static void read_return_apparam(struct obc_session *session,
 				guint16 *phone_book_size, guint8 *new_missed_calls)
 {
@@ -702,6 +662,7 @@ static DBusMessage *pbap_select(DBusConnection *connection,
 {
 	struct pbap_data *pbap = user_data;
 	const char *item, *location;
+	char *path;
 	GError *err = NULL;
 
 	if (dbus_message_get_args(message, NULL,
@@ -711,14 +672,28 @@ static DBusMessage *pbap_select(DBusConnection *connection,
 		return g_dbus_create_error(message,
 				ERROR_INF ".InvalidArguments", NULL);
 
-	if (!pbap_setpath(pbap, location, item, &err)) {
+	path = build_phonebook_path(location, item);
+	if (path == NULL)
+		return g_dbus_create_error(message,
+				ERROR_INF ".InvalidArguments", "Invalid path");
+
+	if (pbap->path != NULL && g_str_equal(pbap->path, path)) {
+		g_free(path);
+		return dbus_message_new_method_return(message);
+	}
+
+	obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap, &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);
 		return reply;
 	}
 
+	g_free(pbap->path);
+	pbap->path = path;
 	pbap->msg = dbus_message_ref(message);
 
 	return NULL;
-- 
1.7.6.5


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

* [PATCH obexd v2 5/6] client: queue transfers in ftp sessions
  2012-02-14 14:47 [PATCH obexd v2 0/6] Several minor fixes Mikel Astiz
                   ` (3 preceding siblings ...)
  2012-02-14 14:47 ` [PATCH obexd v2 4/6] client: fix pbap select when same path given twice Mikel Astiz
@ 2012-02-14 14:47 ` Mikel Astiz
  2012-02-14 14:47 ` [PATCH obexd v2 6/6] client: queue transfers in pbap sessions Mikel Astiz
  2012-02-16  8:01 ` [PATCH obexd v2 0/6] Several minor fixes Luiz Augusto von Dentz
  6 siblings, 0 replies; 9+ messages in thread
From: Mikel Astiz @ 2012-02-14 14:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

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

Previous implementation reported busy when trying to queue several
transfers in the same session.
---
 client/ftp.c |   39 +++++++++++----------------------------
 1 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/client/ftp.c b/client/ftp.c
index 9e3f6b3..9be5d69 100644
--- a/client/ftp.c
+++ b/client/ftp.c
@@ -48,7 +48,6 @@ static DBusConnection *conn = NULL;
 
 struct ftp_data {
 	struct obc_session *session;
-	DBusMessage *msg;
 };
 
 static void async_cb(struct obc_session *session, GError *err,
@@ -176,36 +175,31 @@ static const GMarkupParser parser = {
 static void get_file_callback(struct obc_session *session, GError *err,
 							void *user_data)
 {
-	struct ftp_data *ftp = user_data;
+	DBusMessage *msg = user_data;
 	DBusMessage *reply;
 
-	if (!ftp->msg)
-		return;
-
 	if (err)
-		reply = g_dbus_create_error(ftp->msg,
+		reply = g_dbus_create_error(msg,
 					"org.openobex.Error.Failed",
 					"%s", err->message);
 	else
-		reply = dbus_message_new_method_return(ftp->msg);
+		reply = dbus_message_new_method_return(msg);
 
 	g_dbus_send_message(conn, reply);
-
-	dbus_message_unref(ftp->msg);
-	ftp->msg = NULL;
+	dbus_message_unref(msg);
 }
 
 static void list_folder_callback(struct obc_session *session,
 					GError *err, void *user_data)
 {
-	struct ftp_data *ftp = user_data;
+	DBusMessage *msg = user_data;
 	GMarkupParseContext *ctxt;
 	DBusMessage *reply;
 	DBusMessageIter iter, array;
 	const char *buf;
 	size_t size;
 
-	reply = dbus_message_new_method_return(ftp->msg);
+	reply = dbus_message_new_method_return(msg);
 
 	buf = obc_session_get_buffer(session, &size);
 	if (size == 0)
@@ -224,8 +218,7 @@ static void list_folder_callback(struct obc_session *session,
 
 done:
 	g_dbus_send_message(conn, reply);
-	dbus_message_unref(ftp->msg);
-	ftp->msg = NULL;
+	dbus_message_unref(msg);
 }
 
 static DBusMessage *create_folder(DBusConnection *connection,
@@ -263,18 +256,13 @@ static DBusMessage *list_folder(DBusConnection *connection,
 	struct ftp_data *ftp = user_data;
 	struct obc_session *session = ftp->session;
 
-	if (ftp->msg)
-		return g_dbus_create_error(message,
-				"org.openobex.Error.InProgress",
-				"Transfer in progress");
-
 	if (obc_session_get(session, "x-obex/folder-listing",
-			NULL, NULL, NULL, 0, list_folder_callback, ftp) < 0)
+			NULL, NULL, NULL, 0, list_folder_callback, message) < 0)
 		return g_dbus_create_error(message,
 				"org.openobex.Error.Failed",
 				"Failed");
 
-	ftp->msg = dbus_message_ref(message);
+	dbus_message_ref(message);
 
 	return NULL;
 }
@@ -286,11 +274,6 @@ static DBusMessage *get_file(DBusConnection *connection,
 	struct obc_session *session = ftp->session;
 	const char *target_file, *source_file;
 
-	if (ftp->msg)
-		return g_dbus_create_error(message,
-				"org.openobex.Error.InProgress",
-				"Transfer in progress");
-
 	if (dbus_message_get_args(message, NULL,
 				DBUS_TYPE_STRING, &target_file,
 				DBUS_TYPE_STRING, &source_file,
@@ -299,12 +282,12 @@ static DBusMessage *get_file(DBusConnection *connection,
 				"org.openobex.Error.InvalidArguments", NULL);
 
 	if (obc_session_get(session, NULL, source_file,
-			target_file, NULL, 0, get_file_callback, ftp) < 0)
+			target_file, NULL, 0, get_file_callback, message) < 0)
 		return g_dbus_create_error(message,
 				"org.openobex.Error.Failed",
 				"Failed");
 
-	ftp->msg = dbus_message_ref(message);
+	dbus_message_ref(message);
 
 	return NULL;
 }
-- 
1.7.6.5


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

* [PATCH obexd v2 6/6] client: queue transfers in pbap sessions
  2012-02-14 14:47 [PATCH obexd v2 0/6] Several minor fixes Mikel Astiz
                   ` (4 preceding siblings ...)
  2012-02-14 14:47 ` [PATCH obexd v2 5/6] client: queue transfers in ftp sessions Mikel Astiz
@ 2012-02-14 14:47 ` Mikel Astiz
  2012-02-16  8:01 ` [PATCH obexd v2 0/6] Several minor fixes Luiz Augusto von Dentz
  6 siblings, 0 replies; 9+ messages in thread
From: Mikel Astiz @ 2012-02-14 14:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

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

Previous implementation reported busy when trying to queue several
operations in the same session.
---
 client/pbap.c |  127 +++++++++++++++++++++++++++++---------------------------
 1 files changed, 66 insertions(+), 61 deletions(-)

diff --git a/client/pbap.c b/client/pbap.c
index c4b14a2..eed7fa6 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -127,12 +127,16 @@ static const char *filter_list[] = {
 struct pbap_data {
 	struct obc_session *session;
 	char *path;
-	DBusMessage *msg;
 	guint8 format;
 	guint8 order;
 	uint64_t filter;
 };
 
+struct pending_request {
+	struct pbap_data *pbap;
+	DBusMessage *msg;
+};
+
 struct pullphonebook_apparam {
 	uint8_t     filter_tag;
 	uint8_t     filter_len;
@@ -167,6 +171,24 @@ struct apparam_hdr {
 
 static DBusConnection *conn = NULL;
 
+struct pending_request *pending_request_new(struct pbap_data *pbap,
+						DBusMessage *message)
+{
+	struct pending_request *p;
+
+	p = g_new0(struct pending_request, 1);
+	p->pbap = pbap;
+	p->msg = dbus_message_ref(message);
+
+	return p;
+}
+
+static void pending_request_free(struct pending_request *p)
+{
+	dbus_message_unref(p->msg);
+	g_free(p);
+}
+
 static void listing_element(GMarkupParseContext *ctxt,
 				const gchar *element,
 				const gchar **names,
@@ -252,24 +274,21 @@ static void pbap_reset_path(struct pbap_data *pbap)
 static void pbap_setpath_cb(struct obc_session *session, GError *err,
 							gpointer user_data)
 {
-	struct pbap_data *pbap = user_data;
+	struct pending_request *request = user_data;
+	struct pbap_data *pbap = request->pbap;
 
 	if (err != NULL)
-		pbap_reset_path(user_data);
-
-	if (pbap->msg == NULL)
-		return;
+		pbap_reset_path(pbap);
 
 	if (err) {
-		DBusMessage *reply= g_dbus_create_error(pbap->msg,
+		DBusMessage *reply = g_dbus_create_error(request->msg,
 							ERROR_INF ".Failed",
 							"%s", err->message);
 		g_dbus_send_message(conn, reply);
 	} else
-		g_dbus_send_reply(conn, pbap->msg, DBUS_TYPE_INVALID);
+		g_dbus_send_reply(conn, request->msg, DBUS_TYPE_INVALID);
 
-	dbus_message_unref(pbap->msg);
-	pbap->msg = NULL;
+	pending_request_free(request);
 }
 
 static void read_return_apparam(struct obc_session *session,
@@ -322,22 +341,19 @@ static void read_return_apparam(struct obc_session *session,
 static void pull_phonebook_callback(struct obc_session *session,
 					GError *err, void *user_data)
 {
-	struct pbap_data *pbap = user_data;
+	struct pending_request *request = user_data;
 	DBusMessage *reply;
 	const char *buf;
 	size_t size;
 
-	if (pbap->msg == NULL)
-		return;
-
 	if (err) {
-		reply = g_dbus_create_error(pbap->msg,
+		reply = g_dbus_create_error(request->msg,
 						"org.openobex.Error.Failed",
 						"%s", err->message);
 		goto send;
 	}
 
-	reply = dbus_message_new_method_return(pbap->msg);
+	reply = dbus_message_new_method_return(request->msg);
 
 	buf = obc_session_get_buffer(session, &size);
 	if (size == 0)
@@ -349,29 +365,25 @@ static void pull_phonebook_callback(struct obc_session *session,
 
 send:
 	g_dbus_send_message(conn, reply);
-	dbus_message_unref(pbap->msg);
-	pbap->msg = NULL;
+	pending_request_free(request);
 }
 
 static void phonebook_size_callback(struct obc_session *session,
 					GError *err, void *user_data)
 {
-	struct pbap_data *pbap = user_data;
+	struct pending_request *request = user_data;
 	DBusMessage *reply;
 	guint16 phone_book_size;
 	guint8 new_missed_calls;
 
-	if (pbap->msg == NULL)
-		return;
-
 	if (err) {
-		reply = g_dbus_create_error(pbap->msg,
+		reply = g_dbus_create_error(request->msg,
 						"org.openobex.Error.Failed",
 						"%s", err->message);
 		goto send;
 	}
 
-	reply = dbus_message_new_method_return(pbap->msg);
+	reply = dbus_message_new_method_return(request->msg);
 
 	read_return_apparam(session, &phone_book_size, &new_missed_calls);
 
@@ -381,31 +393,27 @@ static void phonebook_size_callback(struct obc_session *session,
 
 send:
 	g_dbus_send_message(conn, reply);
-	dbus_message_unref(pbap->msg);
-	pbap->msg = NULL;
+	pending_request_free(request);
 }
 
 static void pull_vcard_listing_callback(struct obc_session *session,
 					GError *err, void *user_data)
 {
-	struct pbap_data *pbap = user_data;
+	struct pending_request *request = user_data;
 	GMarkupParseContext *ctxt;
 	DBusMessage *reply;
 	DBusMessageIter iter, array;
 	const char *buf;
 	size_t size;
 
-	if (pbap->msg == NULL)
-		return;
-
 	if (err) {
-		reply = g_dbus_create_error(pbap->msg,
+		reply = g_dbus_create_error(request->msg,
 						"org.openobex.Error.Failed",
 						"%s", err->message);
 		goto send;
 	}
 
-	reply = dbus_message_new_method_return(pbap->msg);
+	reply = dbus_message_new_method_return(request->msg);
 
 	buf = obc_session_get_buffer(session, &size);
 	if (size == 0)
@@ -423,8 +431,7 @@ static void pull_vcard_listing_callback(struct obc_session *session,
 
 send:
 	g_dbus_send_message(conn, reply);
-	dbus_message_unref(pbap->msg);
-	pbap->msg = NULL;
+	pending_request_free(request);
 }
 
 static DBusMessage *pull_phonebook(struct pbap_data *pbap,
@@ -433,14 +440,10 @@ static DBusMessage *pull_phonebook(struct pbap_data *pbap,
 					guint8 format, guint16 maxlistcount,
 					guint16 liststartoffset)
 {
+	struct pending_request *request;
 	struct pullphonebook_apparam apparam;
 	session_callback_t func;
 
-	if (pbap->msg)
-		return g_dbus_create_error(message,
-				"org.openobex.Error.InProgress",
-				"Transfer in progress");
-
 	apparam.filter_tag = FILTER_TAG;
 	apparam.filter_len = FILTER_LEN;
 	apparam.filter = GUINT64_TO_BE(filter);
@@ -466,14 +469,16 @@ static DBusMessage *pull_phonebook(struct pbap_data *pbap,
 		return NULL;
 	}
 
+	request = pending_request_new(pbap, message);
+
 	if (obc_session_get(pbap->session, "x-bt/phonebook", name, NULL,
 				(guint8 *) &apparam, sizeof(apparam),
-				func, pbap) < 0)
+				func, request) < 0) {
+		pending_request_free(request);
 		return g_dbus_create_error(message,
 				"org.openobex.Error.Failed",
 				"Failed");
-
-	pbap->msg = dbus_message_ref(message);
+	}
 
 	return NULL;
 }
@@ -495,15 +500,11 @@ static DBusMessage *pull_vcard_listing(struct pbap_data *pbap,
 					guint8 order, char *searchval, guint8 attrib,
 					guint16 count, guint16 offset)
 {
+	struct pending_request *request;
 	guint8 *p, *apparam = NULL;
 	gint apparam_size;
 	int err;
 
-	if (pbap->msg)
-		return g_dbus_create_error(message,
-				"org.openobex.Error.InProgress",
-				"Transfer in progress");
-
 	/* trunc the searchval string if it's length exceed the max value of guint8 */
 	if (strlen(searchval) > 254)
 		searchval[255] = '\0';
@@ -530,16 +531,18 @@ static DBusMessage *pull_vcard_listing(struct pbap_data *pbap,
 	offset = GUINT16_TO_BE(offset);
 	p = fill_apparam(p, &offset, LISTSTARTOFFSET_TAG, LISTSTARTOFFSET_LEN);
 
+	request = pending_request_new(pbap, message);
+
 	err = obc_session_get(pbap->session, "x-bt/vcard-listing", name, NULL,
 				apparam, apparam_size,
-				pull_vcard_listing_callback, pbap);
+				pull_vcard_listing_callback, request);
 	g_free(apparam);
-	if (err < 0)
+	if (err < 0) {
+		pending_request_free(request);
 		return g_dbus_create_error(message,
 				"org.openobex.Error.Failed",
 				"Failed");
-
-	pbap->msg = dbus_message_ref(message);
+	}
 
 	return NULL;
 }
@@ -663,6 +666,7 @@ static DBusMessage *pbap_select(DBusConnection *connection,
 	struct pbap_data *pbap = user_data;
 	const char *item, *location;
 	char *path;
+	struct pending_request *request;
 	GError *err = NULL;
 
 	if (dbus_message_get_args(message, NULL,
@@ -682,19 +686,22 @@ static DBusMessage *pbap_select(DBusConnection *connection,
 		return dbus_message_new_method_return(message);
 	}
 
-	obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap, &err);
+	request = pending_request_new(pbap, message);
+
+	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;
 	}
 
 	g_free(pbap->path);
 	pbap->path = path;
-	pbap->msg = dbus_message_ref(message);
 
 	return NULL;
 }
@@ -725,6 +732,7 @@ static DBusMessage *pbap_pull_vcard(DBusConnection *connection,
 	struct pbap_data *pbap = user_data;
 	struct pullvcardentry_apparam apparam;
 	const char *name;
+	struct pending_request *request;
 
 	if (!pbap->path)
 		return g_dbus_create_error(message,
@@ -737,11 +745,6 @@ static DBusMessage *pbap_pull_vcard(DBusConnection *connection,
 		return g_dbus_create_error(message,
 				ERROR_INF ".InvalidArguments", NULL);
 
-	if (pbap->msg)
-		return g_dbus_create_error(message,
-				"org.openobex.Error.InProgress",
-				"Transfer in progress");
-
 	apparam.filter_tag = FILTER_TAG;
 	apparam.filter_len = FILTER_LEN;
 	apparam.filter = GUINT64_TO_BE(pbap->filter);
@@ -749,14 +752,16 @@ static DBusMessage *pbap_pull_vcard(DBusConnection *connection,
 	apparam.format_len = FORMAT_LEN;
 	apparam.format = pbap->format;
 
+	request = pending_request_new(pbap, message);
+
 	if (obc_session_get(pbap->session, "x-bt/vcard", name, NULL,
 			(guint8 *)&apparam, sizeof(apparam),
-			pull_phonebook_callback, pbap) < 0)
+			pull_phonebook_callback, request) < 0) {
+		pending_request_free(request);
 		return g_dbus_create_error(message,
 				"org.openobex.Error.Failed",
 				"Failed");
-
-	pbap->msg = dbus_message_ref(message);
+	}
 
 	return NULL;
 }
-- 
1.7.6.5


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

* Re: [PATCH obexd v2 0/6] Several minor fixes
  2012-02-14 14:47 [PATCH obexd v2 0/6] Several minor fixes Mikel Astiz
                   ` (5 preceding siblings ...)
  2012-02-14 14:47 ` [PATCH obexd v2 6/6] client: queue transfers in pbap sessions Mikel Astiz
@ 2012-02-16  8:01 ` Luiz Augusto von Dentz
  6 siblings, 0 replies; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2012-02-16  8:01 UTC (permalink / raw)
  To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz

Hi Mikel,

On Tue, Feb 14, 2012 at 4:47 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> This second series includes the patches acked in the previous proposal (patches 1, 4, 5, 6) and the reviewed patches 2 and 3.
>
> Patch 2 now considers the possibility of unreferencing the session inside the transfer callback.
>
> Patch 3 now follows more strictly the glib convention on error reporting parameters.
>
> Mikel Astiz (6):
>  client: fix memory leak in obc_session_put
>  client: fix unreported error case
>  client: fix incorrect error check
>  client: fix pbap select when same path given twice
>  client: queue transfers in ftp sessions
>  client: queue transfers in pbap sessions
>
>  client/ftp.c     |   39 ++++---------
>  client/pbap.c    |  176 ++++++++++++++++++++++++------------------------------
>  client/session.c |   22 ++++++-
>  3 files changed, 109 insertions(+), 128 deletions(-)
>
> --
> 1.7.6.5

Ack

-- 
Luiz Augusto von Dentz

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

* Re: [PATCH obexd v2 2/6] client: fix unreported error case
  2012-02-14 14:47 ` [PATCH obexd v2 2/6] client: fix unreported error case Mikel Astiz
@ 2012-02-16 13:07   ` Johan Hedberg
  0 siblings, 0 replies; 9+ messages in thread
From: Johan Hedberg @ 2012-02-16 13:07 UTC (permalink / raw)
  To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz

Hi Mikel,

On Tue, Feb 14, 2012, Mikel Astiz wrote:
> +		if (p->func) {
> +			GError *gerr;
> +
> +			g_set_error(&gerr, OBEX_IO_ERROR, err,
> +							"Authorization failed");

This is wrong. You must initialize gerr to NULL before passing a pointer
to it to g_set_error. From the GLib documentation:

	"if err is non-NULL, then *err must be NULL"

Johan

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

end of thread, other threads:[~2012-02-16 13:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-14 14:47 [PATCH obexd v2 0/6] Several minor fixes Mikel Astiz
2012-02-14 14:47 ` [PATCH obexd v2 1/6] client: fix memory leak in obc_session_put Mikel Astiz
2012-02-14 14:47 ` [PATCH obexd v2 2/6] client: fix unreported error case Mikel Astiz
2012-02-16 13:07   ` Johan Hedberg
2012-02-14 14:47 ` [PATCH obexd v2 3/6] client: fix incorrect error check Mikel Astiz
2012-02-14 14:47 ` [PATCH obexd v2 4/6] client: fix pbap select when same path given twice Mikel Astiz
2012-02-14 14:47 ` [PATCH obexd v2 5/6] client: queue transfers in ftp sessions Mikel Astiz
2012-02-14 14:47 ` [PATCH obexd v2 6/6] client: queue transfers in pbap sessions Mikel Astiz
2012-02-16  8:01 ` [PATCH obexd v2 0/6] Several minor fixes Luiz Augusto von Dentz

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).