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