* [PATCH obexd 1/6] client: fix memory leak in obc_session_put
@ 2012-02-13 14:39 Mikel Astiz
2012-02-13 14:39 ` [PATCH obexd 2/6] client: fix unreported error case Mikel Astiz
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Mikel Astiz @ 2012-02-13 14:39 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 585e402..cb46510 100644
--- a/client/session.c
+++ b/client/session.c
@@ -1007,8 +1007,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);
@@ -1016,8 +1018,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] 15+ messages in thread
* [PATCH obexd 2/6] client: fix unreported error case
2012-02-13 14:39 [PATCH obexd 1/6] client: fix memory leak in obc_session_put Mikel Astiz
@ 2012-02-13 14:39 ` Mikel Astiz
2012-02-14 9:11 ` Luiz Augusto von Dentz
2012-02-13 14:39 ` [PATCH obexd 3/6] client: fix incorrect error check Mikel Astiz
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Mikel Astiz @ 2012-02-13 14:39 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 | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/client/session.c b/client/session.c
index cb46510..ea707b3 100644
--- a/client/session.c
+++ b/client/session.c
@@ -734,6 +734,19 @@ static void session_process_queue(struct obc_session *session)
return;
}
+ if (p->func) {
+ GError *gerr;
+
+ obc_session_ref(session);
+ g_set_error(&gerr, OBEX_IO_ERROR, err,
+ "Authorization failed");
+
+ p->func(session, gerr, p->data);
+
+ g_error_free(gerr);
+ obc_session_unref(session);
+ }
+
pending_request_free(p);
}
}
--
1.7.6.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH obexd 3/6] client: fix incorrect error check
2012-02-13 14:39 [PATCH obexd 1/6] client: fix memory leak in obc_session_put Mikel Astiz
2012-02-13 14:39 ` [PATCH obexd 2/6] client: fix unreported error case Mikel Astiz
@ 2012-02-13 14:39 ` Mikel Astiz
2012-02-14 9:16 ` Luiz Augusto von Dentz
2012-02-13 14:39 ` [PATCH obexd 4/6] client: fix pbap select when same path given twice Mikel Astiz
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Mikel Astiz @ 2012-02-13 14:39 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 | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/client/pbap.c b/client/pbap.c
index 4910536..c58557d 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -301,7 +301,7 @@ static gboolean pbap_setpath(struct pbap_data *pbap, const char *location,
}
obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap, err);
- if (err != NULL) {
+ if (*err == NULL) {
g_free(pbap->path);
pbap->path = path;
return TRUE;
--
1.7.6.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH obexd 4/6] client: fix pbap select when same path given twice
2012-02-13 14:39 [PATCH obexd 1/6] client: fix memory leak in obc_session_put Mikel Astiz
2012-02-13 14:39 ` [PATCH obexd 2/6] client: fix unreported error case Mikel Astiz
2012-02-13 14:39 ` [PATCH obexd 3/6] client: fix incorrect error check Mikel Astiz
@ 2012-02-13 14:39 ` Mikel Astiz
2012-02-14 9:17 ` Luiz Augusto von Dentz
2012-02-14 13:01 ` Johan Hedberg
2012-02-13 14:39 ` [PATCH obexd 5/6] client: queue transfers in ftp sessions Mikel Astiz
` (2 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Mikel Astiz @ 2012-02-13 14:39 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 c58557d..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;
- }
-
- obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap, err);
- if (*err == NULL) {
- 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] 15+ messages in thread
* [PATCH obexd 5/6] client: queue transfers in ftp sessions
2012-02-13 14:39 [PATCH obexd 1/6] client: fix memory leak in obc_session_put Mikel Astiz
` (2 preceding siblings ...)
2012-02-13 14:39 ` [PATCH obexd 4/6] client: fix pbap select when same path given twice Mikel Astiz
@ 2012-02-13 14:39 ` Mikel Astiz
2012-02-14 9:23 ` Luiz Augusto von Dentz
2012-02-13 14:39 ` [PATCH obexd 6/6] client: queue transfers in pbap sessions Mikel Astiz
2012-02-14 8:54 ` [PATCH obexd 1/6] client: fix memory leak in obc_session_put Luiz Augusto von Dentz
5 siblings, 1 reply; 15+ messages in thread
From: Mikel Astiz @ 2012-02-13 14:39 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] 15+ messages in thread
* [PATCH obexd 6/6] client: queue transfers in pbap sessions
2012-02-13 14:39 [PATCH obexd 1/6] client: fix memory leak in obc_session_put Mikel Astiz
` (3 preceding siblings ...)
2012-02-13 14:39 ` [PATCH obexd 5/6] client: queue transfers in ftp sessions Mikel Astiz
@ 2012-02-13 14:39 ` Mikel Astiz
2012-02-14 9:24 ` Luiz Augusto von Dentz
2012-02-14 13:03 ` Johan Hedberg
2012-02-14 8:54 ` [PATCH obexd 1/6] client: fix memory leak in obc_session_put Luiz Augusto von Dentz
5 siblings, 2 replies; 15+ messages in thread
From: Mikel Astiz @ 2012-02-13 14:39 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] 15+ messages in thread
* Re: [PATCH obexd 1/6] client: fix memory leak in obc_session_put
2012-02-13 14:39 [PATCH obexd 1/6] client: fix memory leak in obc_session_put Mikel Astiz
` (4 preceding siblings ...)
2012-02-13 14:39 ` [PATCH obexd 6/6] client: queue transfers in pbap sessions Mikel Astiz
@ 2012-02-14 8:54 ` Luiz Augusto von Dentz
5 siblings, 0 replies; 15+ messages in thread
From: Luiz Augusto von Dentz @ 2012-02-14 8:54 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
Hi Mikel,
On Mon, Feb 13, 2012 at 4:39 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> 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 585e402..cb46510 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -1007,8 +1007,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);
>
> @@ -1016,8 +1018,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
Looks good, ack.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH obexd 2/6] client: fix unreported error case
2012-02-13 14:39 ` [PATCH obexd 2/6] client: fix unreported error case Mikel Astiz
@ 2012-02-14 9:11 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 15+ messages in thread
From: Luiz Augusto von Dentz @ 2012-02-14 9:11 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
Hi Mikel,
On Mon, Feb 13, 2012 at 4:39 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> 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 | 13 +++++++++++++
> 1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/client/session.c b/client/session.c
> index cb46510..ea707b3 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -734,6 +734,19 @@ static void session_process_queue(struct obc_session *session)
> return;
> }
>
> + if (p->func) {
> + GError *gerr;
> +
> + obc_session_ref(session);
> + g_set_error(&gerr, OBEX_IO_ERROR, err,
> + "Authorization failed");
> +
> + p->func(session, gerr, p->data);
> +
> + g_error_free(gerr);
> + obc_session_unref(session);
Im afraid the ref need to be taken before the while to protect against
the callback calling session_shutdown and end up freeing the session.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH obexd 3/6] client: fix incorrect error check
2012-02-13 14:39 ` [PATCH obexd 3/6] client: fix incorrect error check Mikel Astiz
@ 2012-02-14 9:16 ` Luiz Augusto von Dentz
2012-02-14 12:59 ` Johan Hedberg
0 siblings, 1 reply; 15+ messages in thread
From: Luiz Augusto von Dentz @ 2012-02-14 9:16 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
Hi Mikel,
On Mon, Feb 13, 2012 at 4:39 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> Previous statement always returned success.
> ---
> client/pbap.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/client/pbap.c b/client/pbap.c
> index 4910536..c58557d 100644
> --- a/client/pbap.c
> +++ b/client/pbap.c
> @@ -301,7 +301,7 @@ static gboolean pbap_setpath(struct pbap_data *pbap, const char *location,
> }
>
> obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap, err);
> - if (err != NULL) {
> + if (*err == NULL) {
> g_free(pbap->path);
> pbap->path = path;
> return TRUE;
> --
> 1.7.6.5
Nice catch, ack.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH obexd 4/6] client: fix pbap select when same path given twice
2012-02-13 14:39 ` [PATCH obexd 4/6] client: fix pbap select when same path given twice Mikel Astiz
@ 2012-02-14 9:17 ` Luiz Augusto von Dentz
2012-02-14 13:01 ` Johan Hedberg
1 sibling, 0 replies; 15+ messages in thread
From: Luiz Augusto von Dentz @ 2012-02-14 9:17 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
Hi Mikel,
On Mon, Feb 13, 2012 at 4:39 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> 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 c58557d..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;
> - }
> -
> - obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap, err);
> - if (*err == NULL) {
> - 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
Looks good, ack
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH obexd 5/6] client: queue transfers in ftp sessions
2012-02-13 14:39 ` [PATCH obexd 5/6] client: queue transfers in ftp sessions Mikel Astiz
@ 2012-02-14 9:23 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 15+ messages in thread
From: Luiz Augusto von Dentz @ 2012-02-14 9:23 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
Hi Mikel,
On Mon, Feb 13, 2012 at 4:39 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> 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
Ack
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH obexd 6/6] client: queue transfers in pbap sessions
2012-02-13 14:39 ` [PATCH obexd 6/6] client: queue transfers in pbap sessions Mikel Astiz
@ 2012-02-14 9:24 ` Luiz Augusto von Dentz
2012-02-14 13:03 ` Johan Hedberg
1 sibling, 0 replies; 15+ messages in thread
From: Luiz Augusto von Dentz @ 2012-02-14 9:24 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
Hi Mikel,
On Mon, Feb 13, 2012 at 4:39 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> 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
>
> --
Ack
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH obexd 3/6] client: fix incorrect error check
2012-02-14 9:16 ` Luiz Augusto von Dentz
@ 2012-02-14 12:59 ` Johan Hedberg
0 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2012-02-14 12:59 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Mikel Astiz, linux-bluetooth, Mikel Astiz
Hi,
On Tue, Feb 14, 2012, Luiz Augusto von Dentz wrote:
> On Mon, Feb 13, 2012 at 4:39 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> > From: Mikel Astiz <mikel.astiz@bmw-carit.de>
> >
> > Previous statement always returned success.
> > ---
> > client/pbap.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/client/pbap.c b/client/pbap.c
> > index 4910536..c58557d 100644
> > --- a/client/pbap.c
> > +++ b/client/pbap.c
> > @@ -301,7 +301,7 @@ static gboolean pbap_setpath(struct pbap_data *pbap, const char *location,
> > }
> >
> > obc_session_setpath(pbap->session, path, pbap_setpath_cb, pbap, err);
> > - if (err != NULL) {
> > + if (*err == NULL) {
> > g_free(pbap->path);
> > pbap->path = path;
> > return TRUE;
> > --
> > 1.7.6.5
>
> Nice catch, ack.
This is still not right. With Mikels patch the code would be trying to
read a pointer value from the memory address 0 if NULL was passed as
err. What you really want is to check for the return value of
obc_session_setpath and if take this if-branch if it is greater than 0
(the function returns a guint), i.e. the value of err doesn't matter
here (since the caller of pbap_setpath is allowed to pass either NULL or
non-NULL).
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH obexd 4/6] client: fix pbap select when same path given twice
2012-02-13 14:39 ` [PATCH obexd 4/6] client: fix pbap select when same path given twice Mikel Astiz
2012-02-14 9:17 ` Luiz Augusto von Dentz
@ 2012-02-14 13:01 ` Johan Hedberg
1 sibling, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2012-02-14 13:01 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
Hi,
On Mon, Feb 13, 2012, Mikel Astiz wrote:
> 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(-)
I can't apply this one because of the fixes needed to the previous one.
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH obexd 6/6] client: queue transfers in pbap sessions
2012-02-13 14:39 ` [PATCH obexd 6/6] client: queue transfers in pbap sessions Mikel Astiz
2012-02-14 9:24 ` Luiz Augusto von Dentz
@ 2012-02-14 13:03 ` Johan Hedberg
1 sibling, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2012-02-14 13:03 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
Hi Mikel,
On Mon, Feb 13, 2012, Mikel Astiz wrote:
> 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(-)
This one doesn't apply either without the other patches that need
fixing. I've applied patches 1 and 5 though so no need to resend them.
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-02-14 13:03 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-13 14:39 [PATCH obexd 1/6] client: fix memory leak in obc_session_put Mikel Astiz
2012-02-13 14:39 ` [PATCH obexd 2/6] client: fix unreported error case Mikel Astiz
2012-02-14 9:11 ` Luiz Augusto von Dentz
2012-02-13 14:39 ` [PATCH obexd 3/6] client: fix incorrect error check Mikel Astiz
2012-02-14 9:16 ` Luiz Augusto von Dentz
2012-02-14 12:59 ` Johan Hedberg
2012-02-13 14:39 ` [PATCH obexd 4/6] client: fix pbap select when same path given twice Mikel Astiz
2012-02-14 9:17 ` Luiz Augusto von Dentz
2012-02-14 13:01 ` Johan Hedberg
2012-02-13 14:39 ` [PATCH obexd 5/6] client: queue transfers in ftp sessions Mikel Astiz
2012-02-14 9:23 ` Luiz Augusto von Dentz
2012-02-13 14:39 ` [PATCH obexd 6/6] client: queue transfers in pbap sessions Mikel Astiz
2012-02-14 9:24 ` Luiz Augusto von Dentz
2012-02-14 13:03 ` Johan Hedberg
2012-02-14 8:54 ` [PATCH obexd 1/6] client: fix memory leak in obc_session_put Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.