2012-02-21 08:46:14

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 0/9] Transfer cancelation

From: Mikel Astiz <[email protected]>

This patch series proposes several fixes regarding the cancelation of transfers.

The goal is that the Cancel method defined in the transfer's D-Bus API should work for any registered transfer, be it in progress or queued. Currently none of them seem to work properly.

Mikel Astiz (9):
gobex: fix callback remove when canceling transfer
client: fix obc_session_get_buffer
client: deny cancel when no agent present
client: remove unnecessary code
client: process transfer queue only if none active
client: terminate queued transfers properly
client: expose obc_transfer_set_callback
client: allow cancelling queued transfers
client: make sure callback does not match size

client/session.c | 58 +++++++++++++++++++++++++++++++++++-----------------
client/transfer.c | 32 ++++++++++++----------------
client/transfer.h | 10 +++++---
gobex/gobex.c | 3 ++
4 files changed, 62 insertions(+), 41 deletions(-)

--
1.7.6.5



2012-02-21 11:56:07

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd 8/9] client: allow cancelling queued transfers

Hi Mikel,

On Tue, Feb 21, 2012 at 10:46 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> The Cancel() method in the D-Bus api should also abort queued transfers,
> which should just be removed from the queue.
> ---
> ?client/transfer.c | ? 12 ++++++++----
> ?1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/client/transfer.c b/client/transfer.c
> index 2cf5786..f380cb7 100644
> --- a/client/transfer.c
> +++ b/client/transfer.c
> @@ -143,11 +143,15 @@ static void obc_transfer_abort(struct obc_transfer *transfer)
> ?{
> ? ? ? ?struct transfer_callback *callback = transfer->callback;
>
> - ? ? ? if (transfer->xfer == 0)
> - ? ? ? ? ? ? ? return;
> + ? ? ? if (transfer->xfer != 0) {
> + ? ? ? ? ? ? ? g_obex_cancel_transfer(transfer->xfer);
> + ? ? ? ? ? ? ? transfer->xfer = 0;
> + ? ? ? }
>
> - ? ? ? g_obex_cancel_transfer(transfer->xfer);
> - ? ? ? transfer->xfer = 0;
> + ? ? ? if (transfer->obex != NULL) {
> + ? ? ? ? ? ? ? g_obex_unref(transfer->obex);
> + ? ? ? ? ? ? ? transfer->obex = NULL;
> + ? ? ? }
>
> ? ? ? ?if (callback) {
> ? ? ? ? ? ? ? ?GError *err;
> --
> 1.7.6.5

Looks, but this is a fix please update the commit message to make it clear.

--
Luiz Augusto von Dentz

2012-02-21 11:51:30

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd 7/9] client: expose obc_transfer_set_callback

Hi Mikel,

On Tue, Feb 21, 2012 at 10:46 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> This will allow setting the callback before the transfer is started,
> particularly to report queued transfer cancellations.

IMO making obc_transfer_register to take the callback and user_pointer
is better, we don't want anything to overwrite the callback.

--
Luiz Augusto von Dentz

2012-02-21 11:40:16

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd 6/9] client: terminate queued transfers properly

Hi Mikel,

On Tue, Feb 21, 2012 at 10:46 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> Previous implementation of session_terminate_transfer assumed that the
> transfer being terminated would always be the active one. However, it
> should be possible to cancel any queued transfer using the D-Bus api.
> ---
> ?client/session.c | ? 32 ++++++++++++++++++++++++--------
> ?1 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/client/session.c b/client/session.c
> index eaabcf9..50a6a5d 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -762,18 +762,34 @@ static void session_terminate_transfer(struct obc_session *session,
> ?{
> ? ? ? ?struct pending_request *p = session->p;
>
> - ? ? ? if (p == NULL || p->transfer != transfer)
> - ? ? ? ? ? ? ? return;
> -
> ? ? ? ?obc_session_ref(session);
>
> - ? ? ? if (p->func)
> - ? ? ? ? ? ? ? p->func(session, gerr, p->data);
> + ? ? ? if (p != NULL && p->transfer == transfer) {
> + ? ? ? ? ? ? ? if (p->func)
> + ? ? ? ? ? ? ? ? ? ? ? p->func(session, gerr, p->data);
>
> - ? ? ? pending_request_free(p);
> - ? ? ? session->p = NULL;
> + ? ? ? ? ? ? ? pending_request_free(p);
> + ? ? ? ? ? ? ? session->p = NULL;

Maybe a goto would be better here, since calling to callback and
freeing the pending request should common.

> - ? ? ? session_process_queue(session);
> + ? ? ? ? ? ? ? session_process_queue(session);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? GList *l;
> +
> + ? ? ? ? ? ? ? for (l = session->queue->head; l; l = l->next) {
> + ? ? ? ? ? ? ? ? ? ? ? p = l->data;
> +
> + ? ? ? ? ? ? ? ? ? ? ? if (p->transfer != transfer)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? continue;
> +
> + ? ? ? ? ? ? ? ? ? ? ? g_queue_delete_link(session->queue, l);
> +
> + ? ? ? ? ? ? ? ? ? ? ? if (p->func)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? p->func(session, gerr, p->data);
> +
> + ? ? ? ? ? ? ? ? ? ? ? pending_request_free(p);
> + ? ? ? ? ? ? ? ? ? ? ? break;
> + ? ? ? ? ? ? ? }
> + ? ? ? }

And here you can make use of g_queue_find_custom like we do in
g_obex_cancel_req.


--
Luiz Augusto von Dentz

2012-02-21 11:27:22

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd 4/9] client: remove unnecessary code

Hi Mikel,

On Tue, Feb 21, 2012 at 10:46 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> No pending requests will exist when session_free is reached, given that
> pending transfes always hold a reference to the session.
> ---
> ?client/session.c | ? ?8 +-------
> ?1 files changed, 1 insertions(+), 7 deletions(-)
>
> diff --git a/client/session.c b/client/session.c
> index e113d1f..160e015 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -179,11 +179,8 @@ static void session_free(struct obc_session *session)
> ? ? ? ? ? ? ? ?obc_agent_free(session->agent);
> ? ? ? ?}
>
> - ? ? ? if (session->queue) {
> - ? ? ? ? ? ? ? g_queue_foreach(session->queue, (GFunc) pending_request_free,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? NULL);
> + ? ? ? if (session->queue)
> ? ? ? ? ? ? ? ?g_queue_free(session->queue);
> - ? ? ? }
>
> ? ? ? ?if (session->watch)
> ? ? ? ? ? ? ? ?g_dbus_remove_watch(session->conn, session->watch);
> @@ -200,9 +197,6 @@ static void session_free(struct obc_session *session)
> ? ? ? ?if (session->conn)
> ? ? ? ? ? ? ? ?dbus_connection_unref(session->conn);
>
> - ? ? ? if (session->p)
> - ? ? ? ? ? ? ? pending_request_free(session->p);
> -
> ? ? ? ?sessions = g_slist_remove(sessions, session);
>
> ? ? ? ?g_free(session->path);
> --
> 1.7.6.5

Maybe some error here would be better, actually it should be fine to
keep as it is to avoid leaking memory and be more robust with changes
in the references, in the other hand printing an error on unref when
it is < 0 should hint that something is broken in refcount.

--
Luiz Augusto von Dentz

2012-02-21 11:16:04

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd 1/9] gobex: fix callback remove when canceling transfer

Hi Mikel,

On Tue, Feb 21, 2012 at 1:02 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Mikel,
>
> On Tue, Feb 21, 2012 at 10:46 AM, Mikel Astiz <[email protected]> wrote:
>> From: Mikel Astiz <[email protected]>
>>
>> This code path could lead to situations where the callback is later
>> used, making the daemon crash.
>> ---
>> ?gobex/gobex.c | ? ?3 +++
>> ?1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/gobex/gobex.c b/gobex/gobex.c
>> index 0665749..bc76e57 100644
>> --- a/gobex/gobex.c
>> +++ b/gobex/gobex.c
>> @@ -735,6 +735,9 @@ gboolean g_obex_cancel_req(GObex *obex, guint req_id, gboolean remove_callback)
>> ? ? ? ? ? ? ? ? ? ? ? ?goto immediate_completion;
>> ? ? ? ? ? ? ? ?}
>>
>> + ? ? ? ? ? ? ? if (remove_callback)
>> + ? ? ? ? ? ? ? ? ? ? ? obex->pending_req->rsp_func = NULL;
>> +
>
> I see the problem, so we are keeping the pending request because of
> the abort response, Im just not sure why we need to report an error if
> cancelled is set on handle_response. So perhaps the right place to fix
> this is in handle_response as soon as it checks for cancelled it
> should skip to the point it frees the pending call, in other words we
> should never need to report G_OBEX_ERROR_CANCELLED for requests since
> we are the ones cancelling them.

Actually this is correct, since when remove_callback is FALSE we still
call the callback so G_OBEX_ERROR_CANCELLED makes sense.

Ack.

--
Luiz Augusto von Dentz

2012-02-21 11:12:28

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd 3/9] client: deny cancel when no agent present

Hi Mikel,

On Tue, Feb 21, 2012 at 10:46 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> The authorization check should consider the scenario of no agent being
> assigned to the transfer.
> ---
> ?client/transfer.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/client/transfer.c b/client/transfer.c
> index 157811d..4054799 100644
> --- a/client/transfer.c
> +++ b/client/transfer.c
> @@ -168,7 +168,7 @@ static DBusMessage *obc_transfer_cancel(DBusConnection *connection,
> ? ? ? ?DBusMessage *reply;
>
> ? ? ? ?sender = dbus_message_get_sender(message);
> - ? ? ? if (g_str_equal(transfer->agent, sender) == FALSE)
> + ? ? ? if (g_strcmp0(transfer->agent, sender) != 0)
> ? ? ? ? ? ? ? ?return g_dbus_create_error(message,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"org.openobex.Error.NotAuthorized",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Not Authorized");
> --
> 1.7.6.5

This is actually a fix, g_str_equal does not do NULL-safe comparisons,
could you please make this clear in your commit message.

--
Luiz Augusto von Dentz

2012-02-21 11:07:53

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd 2/9] client: fix obc_session_get_buffer

Hi Mikel,

On Tue, Feb 21, 2012 at 10:46 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> Size 0 should be reported if no transfer exists. Some existing code
> relies on this behavior.
> ---
> ?client/session.c | ? ?6 +++++-
> ?1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/client/session.c b/client/session.c
> index 85f466a..e113d1f 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -1128,8 +1128,12 @@ const char *obc_session_get_buffer(struct obc_session *session, size_t *size)
> ? ? ? ?const char *buf;
>
> ? ? ? ?transfer = obc_session_get_transfer(session);
> - ? ? ? if (transfer == NULL)
> + ? ? ? if (transfer == NULL) {
> + ? ? ? ? ? ? ? if (size != NULL)
> + ? ? ? ? ? ? ? ? ? ? ? *size = 0;
> +
> ? ? ? ? ? ? ? ?return NULL;
> + ? ? ? }
>
> ? ? ? ?buf = obc_transfer_get_buffer(transfer, size);
>
> --
> 1.7.6.5

Nice catch, ack.

--
Luiz Augusto von Dentz

2012-02-21 11:02:45

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd 1/9] gobex: fix callback remove when canceling transfer

Hi Mikel,

On Tue, Feb 21, 2012 at 10:46 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> This code path could lead to situations where the callback is later
> used, making the daemon crash.
> ---
> ?gobex/gobex.c | ? ?3 +++
> ?1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/gobex/gobex.c b/gobex/gobex.c
> index 0665749..bc76e57 100644
> --- a/gobex/gobex.c
> +++ b/gobex/gobex.c
> @@ -735,6 +735,9 @@ gboolean g_obex_cancel_req(GObex *obex, guint req_id, gboolean remove_callback)
> ? ? ? ? ? ? ? ? ? ? ? ?goto immediate_completion;
> ? ? ? ? ? ? ? ?}
>
> + ? ? ? ? ? ? ? if (remove_callback)
> + ? ? ? ? ? ? ? ? ? ? ? obex->pending_req->rsp_func = NULL;
> +

I see the problem, so we are keeping the pending request because of
the abort response, Im just not sure why we need to report an error if
cancelled is set on handle_response. So perhaps the right place to fix
this is in handle_response as soon as it checks for cancelled it
should skip to the point it frees the pending call, in other words we
should never need to report G_OBEX_ERROR_CANCELLED for requests since
we are the ones cancelling them.

--
Luiz Augusto von Dentz

2012-02-21 08:46:23

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 9/9] client: make sure callback does not match size

From: Mikel Astiz <[email protected]>

Otherwise it can be interpreted as successfully finished, which has its
own code path.
---
client/transfer.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index f380cb7..99007ec 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -395,7 +395,7 @@ static void get_buf_xfer_progress(GObex *obex, GError *err, GObexPacket *rsp,
transfer, &err);
}

- if (callback)
+ if (callback && transfer->transferred != transfer->size)
callback->func(transfer, transfer->transferred, err,
callback->data);
}
@@ -439,7 +439,7 @@ static gboolean get_xfer_progress(const void *buf, gsize len,
transfer->filled -= w;
}

- if (callback)
+ if (callback && transfer->transferred != transfer->size)
callback->func(transfer, transfer->transferred, NULL,
callback->data);

--
1.7.6.5


2012-02-21 08:46:20

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 6/9] client: terminate queued transfers properly

From: Mikel Astiz <[email protected]>

Previous implementation of session_terminate_transfer assumed that the
transfer being terminated would always be the active one. However, it
should be possible to cancel any queued transfer using the D-Bus api.
---
client/session.c | 32 ++++++++++++++++++++++++--------
1 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/client/session.c b/client/session.c
index eaabcf9..50a6a5d 100644
--- a/client/session.c
+++ b/client/session.c
@@ -762,18 +762,34 @@ static void session_terminate_transfer(struct obc_session *session,
{
struct pending_request *p = session->p;

- if (p == NULL || p->transfer != transfer)
- return;
-
obc_session_ref(session);

- if (p->func)
- p->func(session, gerr, p->data);
+ if (p != NULL && p->transfer == transfer) {
+ if (p->func)
+ p->func(session, gerr, p->data);

- pending_request_free(p);
- session->p = NULL;
+ pending_request_free(p);
+ session->p = NULL;

- session_process_queue(session);
+ session_process_queue(session);
+ } else {
+ GList *l;
+
+ for (l = session->queue->head; l; l = l->next) {
+ p = l->data;
+
+ if (p->transfer != transfer)
+ continue;
+
+ g_queue_delete_link(session->queue, l);
+
+ if (p->func)
+ p->func(session, gerr, p->data);
+
+ pending_request_free(p);
+ break;
+ }
+ }

obc_session_unref(session);
}
--
1.7.6.5


2012-02-21 08:46:19

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 5/9] client: process transfer queue only if none active

From: Mikel Astiz <[email protected]>

session_process_queue should make sure there is no active operation, to
avoid starting a second one at the same time.
---
client/session.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/client/session.c b/client/session.c
index 160e015..eaabcf9 100644
--- a/client/session.c
+++ b/client/session.c
@@ -726,7 +726,8 @@ static void session_process_queue(struct obc_session *session)
{
struct pending_request *p;

- if (session->queue == NULL || g_queue_is_empty(session->queue))
+ if (session->queue == NULL || g_queue_is_empty(session->queue) ||
+ session->p != NULL)
return;

obc_session_ref(session);
--
1.7.6.5


2012-02-21 08:46:21

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 7/9] client: expose obc_transfer_set_callback

From: Mikel Astiz <[email protected]>

This will allow setting the callback before the transfer is started,
particularly to report queued transfer cancellations.
---
client/session.c | 9 +++++++--
client/transfer.c | 14 +++-----------
client/transfer.h | 10 ++++++----
3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/client/session.c b/client/session.c
index 50a6a5d..e9cc3ee 100644
--- a/client/session.c
+++ b/client/session.c
@@ -107,6 +107,9 @@ static void session_prepare_put(gpointer data, gpointer user_data);
static void session_terminate_transfer(struct obc_session *session,
struct obc_transfer *transfer,
GError *gerr);
+static void transfer_progress(struct obc_transfer *transfer,
+ gint64 transferred, GError *err,
+ void *user_data);

GQuark obex_io_error_quark(void)
{
@@ -704,6 +707,8 @@ static int session_request(struct obc_session *session,
struct pending_request *p;
int err;

+ obc_transfer_set_callback(transfer, transfer_progress, session);
+
p = pending_request_new(session, transfer, auth_complete, func, data);

if (session->p) {
@@ -877,7 +882,7 @@ static void session_prepare_get(gpointer data, gpointer user_data)
struct obc_transfer *transfer = user_data;
int ret;

- ret = obc_transfer_get(transfer, transfer_progress, session);
+ ret = obc_transfer_get(transfer);
if (ret < 0) {
GError *gerr = NULL;

@@ -1022,7 +1027,7 @@ static void session_prepare_put(gpointer data, gpointer user_data)
struct obc_transfer *transfer = user_data;
int ret;

- ret = obc_transfer_put(transfer, transfer_progress, session);
+ ret = obc_transfer_put(transfer);
if (ret < 0) {
GError *gerr = NULL;

diff --git a/client/transfer.c b/client/transfer.c
index 4054799..2cf5786 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -488,7 +488,7 @@ static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
return size;
}

-static void obc_transfer_set_callback(struct obc_transfer *transfer,
+void obc_transfer_set_callback(struct obc_transfer *transfer,
transfer_callback_t func,
void *user_data)
{
@@ -503,8 +503,7 @@ static void obc_transfer_set_callback(struct obc_transfer *transfer,
transfer->callback = callback;
}

-int obc_transfer_get(struct obc_transfer *transfer, transfer_callback_t func,
- void *user_data)
+int obc_transfer_get(struct obc_transfer *transfer)
{
GError *err = NULL;
GObexPacket *req;
@@ -558,14 +557,10 @@ int obc_transfer_get(struct obc_transfer *transfer, transfer_callback_t func,
if (transfer->xfer == 0)
return -ENOTCONN;

- if (func)
- obc_transfer_set_callback(transfer, func, user_data);
-
return 0;
}

-int obc_transfer_put(struct obc_transfer *transfer, transfer_callback_t func,
- void *user_data)
+int obc_transfer_put(struct obc_transfer *transfer)
{
GError *err = NULL;
GObexPacket *req;
@@ -606,9 +601,6 @@ done:
if (transfer->xfer == 0)
return -ENOTCONN;

- if (func)
- obc_transfer_set_callback(transfer, func, user_data);
-
return 0;
}

diff --git a/client/transfer.h b/client/transfer.h
index e5387fc..348c17c 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -42,10 +42,12 @@ struct obc_transfer *obc_transfer_register(DBusConnection *conn,

void obc_transfer_unregister(struct obc_transfer *transfer);

-int obc_transfer_get(struct obc_transfer *transfer, transfer_callback_t func,
- void *user_data);
-int obc_transfer_put(struct obc_transfer *transfer, transfer_callback_t func,
- void *user_data);
+void obc_transfer_set_callback(struct obc_transfer *transfer,
+ transfer_callback_t func,
+ void *user_data);
+
+int obc_transfer_get(struct obc_transfer *transfer);
+int obc_transfer_put(struct obc_transfer *transfer);

int obc_transfer_get_params(struct obc_transfer *transfer,
struct obc_transfer_params *params);
--
1.7.6.5


2012-02-21 08:46:22

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 8/9] client: allow cancelling queued transfers

From: Mikel Astiz <[email protected]>

The Cancel() method in the D-Bus api should also abort queued transfers,
which should just be removed from the queue.
---
client/transfer.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index 2cf5786..f380cb7 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -143,11 +143,15 @@ static void obc_transfer_abort(struct obc_transfer *transfer)
{
struct transfer_callback *callback = transfer->callback;

- if (transfer->xfer == 0)
- return;
+ if (transfer->xfer != 0) {
+ g_obex_cancel_transfer(transfer->xfer);
+ transfer->xfer = 0;
+ }

- g_obex_cancel_transfer(transfer->xfer);
- transfer->xfer = 0;
+ if (transfer->obex != NULL) {
+ g_obex_unref(transfer->obex);
+ transfer->obex = NULL;
+ }

if (callback) {
GError *err;
--
1.7.6.5


2012-02-21 08:46:17

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 3/9] client: deny cancel when no agent present

From: Mikel Astiz <[email protected]>

The authorization check should consider the scenario of no agent being
assigned to the transfer.
---
client/transfer.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index 157811d..4054799 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -168,7 +168,7 @@ static DBusMessage *obc_transfer_cancel(DBusConnection *connection,
DBusMessage *reply;

sender = dbus_message_get_sender(message);
- if (g_str_equal(transfer->agent, sender) == FALSE)
+ if (g_strcmp0(transfer->agent, sender) != 0)
return g_dbus_create_error(message,
"org.openobex.Error.NotAuthorized",
"Not Authorized");
--
1.7.6.5


2012-02-21 08:46:18

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 4/9] client: remove unnecessary code

From: Mikel Astiz <[email protected]>

No pending requests will exist when session_free is reached, given that
pending transfes always hold a reference to the session.
---
client/session.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/client/session.c b/client/session.c
index e113d1f..160e015 100644
--- a/client/session.c
+++ b/client/session.c
@@ -179,11 +179,8 @@ static void session_free(struct obc_session *session)
obc_agent_free(session->agent);
}

- if (session->queue) {
- g_queue_foreach(session->queue, (GFunc) pending_request_free,
- NULL);
+ if (session->queue)
g_queue_free(session->queue);
- }

if (session->watch)
g_dbus_remove_watch(session->conn, session->watch);
@@ -200,9 +197,6 @@ static void session_free(struct obc_session *session)
if (session->conn)
dbus_connection_unref(session->conn);

- if (session->p)
- pending_request_free(session->p);
-
sessions = g_slist_remove(sessions, session);

g_free(session->path);
--
1.7.6.5


2012-02-21 08:46:15

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 1/9] gobex: fix callback remove when canceling transfer

From: Mikel Astiz <[email protected]>

This code path could lead to situations where the callback is later
used, making the daemon crash.
---
gobex/gobex.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gobex/gobex.c b/gobex/gobex.c
index 0665749..bc76e57 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -735,6 +735,9 @@ gboolean g_obex_cancel_req(GObex *obex, guint req_id, gboolean remove_callback)
goto immediate_completion;
}

+ if (remove_callback)
+ obex->pending_req->rsp_func = NULL;
+
return TRUE;
}

--
1.7.6.5


2012-02-21 08:46:16

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd 2/9] client: fix obc_session_get_buffer

From: Mikel Astiz <[email protected]>

Size 0 should be reported if no transfer exists. Some existing code
relies on this behavior.
---
client/session.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/client/session.c b/client/session.c
index 85f466a..e113d1f 100644
--- a/client/session.c
+++ b/client/session.c
@@ -1128,8 +1128,12 @@ const char *obc_session_get_buffer(struct obc_session *session, size_t *size)
const char *buf;

transfer = obc_session_get_transfer(session);
- if (transfer == NULL)
+ if (transfer == NULL) {
+ if (size != NULL)
+ *size = 0;
+
return NULL;
+ }

buf = obc_transfer_get_buffer(transfer, size);

--
1.7.6.5