2011-01-13 10:52:04

by Slawomir Bochenski

[permalink] [raw]
Subject: [PATCH] Fix plugin close & disconnect functions call order

From: Slawomir Bochenski <[email protected]>

Normally during an OBEX session, calling sequence

service->connect - [driver->open - driver->close]* - service->disconnect

is kept. The only exception to this when the connection is reset
(when no ABORT was sent) during transfer. Then the sequence is:

service->connect - [driver->open - driver->close]* - driver->open -
service->disconnect - driver->close

This patch fixes it, so memory managament of session/transfer data in
service plugin can be easier.
---
src/obex.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/obex.c b/src/obex.c
index 65f17fc..67684fa 100644
--- a/src/obex.c
+++ b/src/obex.c
@@ -1231,6 +1231,8 @@ static void obex_handle_destroy(void *user_data)

os = OBEX_GetUserData(obex);

+ os_reset_session(os);
+
if (os->service && os->service->disconnect)
os->service->disconnect(os, os->service_data);

--
1.7.1



2011-01-14 07:59:00

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] Fix plugin close & disconnect functions call order

Hi,

On Fri, Jan 14, 2011 at 9:13 AM, Slawomir Bochenski <[email protected]> wrote:
> On Thu, Jan 13, 2011 at 6:15 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi,
>>
>> On Thu, Jan 13, 2011 at 12:52 PM, ?<[email protected]> wrote:
>>> From: Slawomir Bochenski <[email protected]>
>>>
>>> Normally during an OBEX session, calling sequence
>>>
>>> service->connect - [driver->open - driver->close]* - service->disconnect
>>>
>>> is kept. The only exception to this when the connection is reset
>>> (when no ABORT was sent) during transfer. Then the sequence is:
>>>
>>> service->connect - [driver->open - driver->close]* - driver->open -
>>> service->disconnect - driver->close
>>>
>>> This patch fixes it, so memory managament of session/transfer data in
>>> service plugin can be easier.
>>> ---
>>> ?src/obex.c | ? ?2 ++
>>> ?1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/src/obex.c b/src/obex.c
>>> index 65f17fc..67684fa 100644
>>> --- a/src/obex.c
>>> +++ b/src/obex.c
>>> @@ -1231,6 +1231,8 @@ static void obex_handle_destroy(void *user_data)
>>>
>>> ? ? ? ?os = OBEX_GetUserData(obex);
>>>
>>> + ? ? ? os_reset_session(os);
>>> +
>>> ? ? ? ?if (os->service && os->service->disconnect)
>>> ? ? ? ? ? ? ? ?os->service->disconnect(os, os->service_data);
>>
>> Looks like a good idea, what about removing from os_session_free?
>>
>>
>> --
>> Luiz Augusto von Dentz
>> Computer Engineer
>>
>
> It looks to me that it could be safely removed from
> obex_session_free(), as when obex_session_free() is called from
> obex_session_start(), struct obex_session *os is in state in which it
> does not need reset. In fact it would be required to remove it from
> there, otherwise service->reset() would be called twice. The only side
> effect is that in case of early error in obex_session_start(),
> service->reset() will not be called at all. So the question is whether
> this last behaviour is really desired (no other service function
> indicating session existence is called before this point)?

If the service was never connected than it is probably ok, actually it
seems broken now because reset can be called after disconnect which is
probably useless.


--
Luiz Augusto von Dentz
Computer Engineer

2011-01-14 07:13:38

by Slawomir Bochenski

[permalink] [raw]
Subject: Re: [PATCH] Fix plugin close & disconnect functions call order

On Thu, Jan 13, 2011 at 6:15 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi,
>
> On Thu, Jan 13, 2011 at 12:52 PM, ?<[email protected]> wrote:
>> From: Slawomir Bochenski <[email protected]>
>>
>> Normally during an OBEX session, calling sequence
>>
>> service->connect - [driver->open - driver->close]* - service->disconnect
>>
>> is kept. The only exception to this when the connection is reset
>> (when no ABORT was sent) during transfer. Then the sequence is:
>>
>> service->connect - [driver->open - driver->close]* - driver->open -
>> service->disconnect - driver->close
>>
>> This patch fixes it, so memory managament of session/transfer data in
>> service plugin can be easier.
>> ---
>> ?src/obex.c | ? ?2 ++
>> ?1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/obex.c b/src/obex.c
>> index 65f17fc..67684fa 100644
>> --- a/src/obex.c
>> +++ b/src/obex.c
>> @@ -1231,6 +1231,8 @@ static void obex_handle_destroy(void *user_data)
>>
>> ? ? ? ?os = OBEX_GetUserData(obex);
>>
>> + ? ? ? os_reset_session(os);
>> +
>> ? ? ? ?if (os->service && os->service->disconnect)
>> ? ? ? ? ? ? ? ?os->service->disconnect(os, os->service_data);
>
> Looks like a good idea, what about removing from os_session_free?
>
>
> --
> Luiz Augusto von Dentz
> Computer Engineer
>

It looks to me that it could be safely removed from
obex_session_free(), as when obex_session_free() is called from
obex_session_start(), struct obex_session *os is in state in which it
does not need reset. In fact it would be required to remove it from
there, otherwise service->reset() would be called twice. The only side
effect is that in case of early error in obex_session_start(),
service->reset() will not be called at all. So the question is whether
this last behaviour is really desired (no other service function
indicating session existence is called before this point)?

--
Slawomir Bochenski

2011-01-13 17:15:58

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] Fix plugin close & disconnect functions call order

Hi,

On Thu, Jan 13, 2011 at 12:52 PM, <[email protected]> wrote:
> From: Slawomir Bochenski <[email protected]>
>
> Normally during an OBEX session, calling sequence
>
> service->connect - [driver->open - driver->close]* - service->disconnect
>
> is kept. The only exception to this when the connection is reset
> (when no ABORT was sent) during transfer. Then the sequence is:
>
> service->connect - [driver->open - driver->close]* - driver->open -
> service->disconnect - driver->close
>
> This patch fixes it, so memory managament of session/transfer data in
> service plugin can be easier.
> ---
> ?src/obex.c | ? ?2 ++
> ?1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/src/obex.c b/src/obex.c
> index 65f17fc..67684fa 100644
> --- a/src/obex.c
> +++ b/src/obex.c
> @@ -1231,6 +1231,8 @@ static void obex_handle_destroy(void *user_data)
>
> ? ? ? ?os = OBEX_GetUserData(obex);
>
> + ? ? ? os_reset_session(os);
> +
> ? ? ? ?if (os->service && os->service->disconnect)
> ? ? ? ? ? ? ? ?os->service->disconnect(os, os->service_data);

Looks like a good idea, what about removing from os_session_free?


--
Luiz Augusto von Dentz
Computer Engineer