2012-01-06 13:31:56

by Divya Yadav

[permalink] [raw]
Subject: [PATCH obexd] MAP: Add UpdateInbox Function

UpdateInbox function allows remote device to initiate an update of
the MSE inbox, i.e. the MSE shall contact the network to retrieve
new messages if available. If MSE does not support the network
update it shall answer with a 'Not implemented' error response.
---
plugins/mas.c | 37 ++++++++++++++++++++++++++++++++++++-
plugins/messages-dummy.c | 7 +++++++
plugins/messages-tracker.c | 7 +++++++
plugins/messages.h | 13 +++++++++++++
4 files changed, 63 insertions(+), 1 deletions(-)

diff --git a/plugins/mas.c b/plugins/mas.c
index 3c3104c..38842e0 100644
--- a/plugins/mas.c
+++ b/plugins/mas.c
@@ -406,6 +406,20 @@ proceed:
obex_object_set_io_flags(mas, G_IO_IN, err);
}

+static void update_inbox_cb(void *session, int err, void *user_data)
+{
+ struct mas_session *mas = user_data;
+
+ DBG("");
+
+ mas->finished = TRUE;
+
+ if (err < 0)
+ obex_object_set_io_flags(mas, G_IO_ERR, err);
+ else
+ obex_object_set_io_flags(mas, G_IO_OUT, 0);
+}
+
static int mas_setpath(struct obex_session *os, void *user_data)
{
const char *name;
@@ -503,6 +517,27 @@ static void *message_open(const char *name, int oflag, mode_t mode,
return mas;
}

+static void *message_update_open(const char *name, int oflag, mode_t mode,
+ void *driver_data, size_t *size,
+ int *err)
+{
+ struct mas_session *mas = driver_data;
+
+ DBG("");
+
+ if (oflag != O_WRONLY) {
+ *err = -EBADR;
+ return NULL;
+ }
+
+ *err = messages_update_inbox(mas->backend_data, update_inbox_cb, mas);
+
+ if (*err < 0)
+ return NULL;
+ else
+ return mas;
+}
+
static void *any_open(const char *name, int oflag, mode_t mode,
void *driver_data, size_t *size, int *err)
{
@@ -628,7 +663,7 @@ static struct obex_mime_type_driver mime_message_update = {
.target = MAS_TARGET,
.target_size = TARGET_SIZE,
.mimetype = "x-bt/MAP-messageUpdate",
- .open = any_open,
+ .open = message_update_open,
.close = any_close,
.read = any_read,
.write = any_write,
diff --git a/plugins/messages-dummy.c b/plugins/messages-dummy.c
index e96b13b..ef1d946 100644
--- a/plugins/messages-dummy.c
+++ b/plugins/messages-dummy.c
@@ -170,6 +170,13 @@ int messages_get_message(void *session,
return -EINVAL;
}

+int messages_update_inbox(void *session,
+ messages_update_inbox_cb callback,
+ void *user_data)
+{
+ return -EINVAL;
+}
+
void messages_abort(void *session)
{
}
diff --git a/plugins/messages-tracker.c b/plugins/messages-tracker.c
index 8f9b30a..713df0c 100644
--- a/plugins/messages-tracker.c
+++ b/plugins/messages-tracker.c
@@ -321,6 +321,13 @@ int messages_get_message(void *session,
return -EINVAL;
}

+int messages_update_inbox(void *session,
+ messages_update_inbox_cb callback,
+ void *user_data)
+{
+ return -EINVAL;
+}
+
void messages_abort(void *session)
{
}
diff --git a/plugins/messages.h b/plugins/messages.h
index 6982edd..705615e 100644
--- a/plugins/messages.h
+++ b/plugins/messages.h
@@ -268,6 +268,19 @@ int messages_get_message(void *session,
messages_get_message_cb callback,
void *user_data);

+/* Informs Message Server to Update Inbox via network.
+ *
+ * session: Backend session.
+ * user_data: User data if any to be sent.
+ * Callback shall be called for every update inbox request recieved from MCE.
+ */
+typedef void (*messages_update_inbox_cb)(void *session, int err,
+ void *user_data);
+
+int messages_update_inbox(void *session,
+ messages_update_inbox_cb callback,
+ void *user_data);
+
/* Aborts currently pending request.
*
* session: Backend session.
--
1.7.0.4



2012-01-09 13:07:35

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH obexd] MAP: Add UpdateInbox Function

Hi Divya,

On Fri, Jan 06, 2012, Divya Yadav wrote:
> UpdateInbox function allows remote device to initiate an update of
> the MSE inbox, i.e. the MSE shall contact the network to retrieve
> new messages if available. If MSE does not support the network
> update it shall answer with a 'Not implemented' error response.
> ---
> plugins/mas.c | 37 ++++++++++++++++++++++++++++++++++++-
> plugins/messages-dummy.c | 7 +++++++
> plugins/messages-tracker.c | 7 +++++++
> plugins/messages.h | 13 +++++++++++++
> 4 files changed, 63 insertions(+), 1 deletions(-)

Applied. Thanks.

Johan

2012-01-05 11:40:54

by Divya Yadav

[permalink] [raw]
Subject: Re: [PATCH obexd] MAP: Add UpdateInbox Function

Hello,

--------------------------------------------------
From: "Slawomir Bochenski" <[email protected]>
Sent: Thursday, January 05, 2012 4:45 PM
To: "Divya Yadav" <[email protected]>
Cc: <[email protected]>
Subject: Re: [PATCH obexd] MAP: Add UpdateInbox Function

> Hi!
>
> On Thu, Jan 5, 2012 at 10:43 AM, Divya Yadav <[email protected]>
> wrote:
>> UpdateInbox function allows remote device to initiate an update of
>> the server's inbox, i.e. the Map server shall contact the network
>
> Maybe better to call it "MSE inbox".
>
>> to retrieve new messages if available. If the server does not allow
>> the network update it shall answer with a 'Not implemented'
>> error response.
>
> More like "does not support" than "does not allow".
>
>> ---
>> plugins/mas.c | 32 +++++++++++++++++++++++++++++++-
>> plugins/messages-dummy.c | 7 +++++++
>> plugins/messages-tracker.c | 10 +++++++++-
>> plugins/messages.h | 14 ++++++++++++++
>> 4 files changed, 61 insertions(+), 2 deletions(-)
>>
>> diff --git a/plugins/mas.c b/plugins/mas.c
>> index 3c3104c..639d0b6 100644
>> --- a/plugins/mas.c
>> +++ b/plugins/mas.c
>> @@ -503,6 +503,36 @@ static void *message_open(const char *name, int
>> oflag, mode_t mode,
>> return mas;
>> }
>>
>> +static void inbox_update_cb(void *session, int err, void *user_data)
>
> Callbacks added previously are all following each other, before the
> section with *_open functions, so I'd rather move this one up. Also
> I'd rather call it update_inbox_cb(), to be consistent with the
> typedef.
>
>> +{
>> + struct mas_session *mas = user_data;
>> +
>> + DBG("");
>> +
>> + mas->finished = TRUE;
>> +
>> + if (err < 0)
>> + obex_object_set_io_flags(mas, G_IO_ERR, err);
>> + else
>> + obex_object_set_io_flags(mas, G_IO_OUT, 0);
>> +}
>> +
>> +static void *message_update_open(const char *name, int oflag, mode_t
>> mode,
>> + void *driver_data, size_t *size, int
>> *err)
>> +{
>> + struct mas_session *mas = driver_data;
>> +
>> + DBG("");
>> +
>> + *err = messages_update_inbox(mas->backend_data, inbox_update_cb,
>> mas);
>> +
>> + if (*err < 0) {
>> + mas->finished = TRUE;
>
> No need for this. If open returns NULL, no read/write is going to be
> called.
>
>> + return NULL;
>> + } else
>> + return mas;
>> +}
>
> This function is missing check for oflag value - this MIME type should
> return error on GET.
>
>> +
>> static void *any_open(const char *name, int oflag, mode_t mode,
>> void *driver_data, size_t *size, int *err)
>> {
>> @@ -628,7 +658,7 @@ static struct obex_mime_type_driver
>> mime_message_update = {
>> .target = MAS_TARGET,
>> .target_size = TARGET_SIZE,
>> .mimetype = "x-bt/MAP-messageUpdate",
>> - .open = any_open,
>> + .open = message_update_open,
>> .close = any_close,
>> .read = any_read,
>> .write = any_write,
>> diff --git a/plugins/messages-dummy.c b/plugins/messages-dummy.c
>> index e96b13b..58f04f6 100644
>> --- a/plugins/messages-dummy.c
>> +++ b/plugins/messages-dummy.c
>> @@ -173,3 +173,10 @@ int messages_get_message(void *session,
>> void messages_abort(void *session)
>> {
>> }
>> +
>> +int messages_update_inbox(void *session,
>> + messages_update_inbox_cb callback,
>> + void *user_data)
>> +{
>> + return -EINVAL;
>> +}
>
> Put this before messages_abort().
>
>> diff --git a/plugins/messages-tracker.c b/plugins/messages-tracker.c
>> index 8f9b30a..6a68b5b 100644
>> --- a/plugins/messages-tracker.c
>> +++ b/plugins/messages-tracker.c
>> @@ -237,7 +237,8 @@ int messages_set_folder(void *s, const char *name,
>> gboolean cdup)
>> return 0;
>> }
>>
>> -static gboolean async_get_folder_listing(void *s) {
>> +static gboolean async_get_folder_listing(void *s)
>> +{
>
> This belongs into a separate patch.
>
>> struct session *session = s;
>> gboolean count = FALSE;
>> int folder_count = 0;
>> @@ -324,3 +325,10 @@ int messages_get_message(void *session,
>> void messages_abort(void *session)
>> {
>> }
>> +
>> +int messages_update_inbox(void *session,
>> + messages_update_inbox_cb callback,
>> + void *user_data)
>> +{
>> + return -EINVAL;
>> +}
>
> Same as for messages-dummy.c.
>
>> diff --git a/plugins/messages.h b/plugins/messages.h
>> index 6982edd..ea231a3 100644
>> --- a/plugins/messages.h
>> +++ b/plugins/messages.h
>> @@ -273,3 +273,17 @@ int messages_get_message(void *session,
>> * session: Backend session.
>> */
>> void messages_abort(void *session);
>> +
>> +/* Informs Message Server to Update Inbox via network
>
> One space after asterisk, dot at the end.
>
>> + *
>> + * session: Backend session.
>> + * user_data: User data if any to be sent
>
> Missing new line. Also missing dot at the end of second line.
>
>> + * Callback shall be called for every update inbox request, where if any
>> error
>> + * should be reported to the remote end.
>
> "shall be called once" maybe. Second part not very understandable. I
> also guess you mean error during initiating inbox update, because the
> update procedure itself should take place after UpdateInbox MAP
> request returns.
>
>> + */
>> +typedef void (*messages_update_inbox_cb)(void *session, int err,
>> + void *user_data);
>> +
>> +int messages_update_inbox(void *session,
>> + messages_update_inbox_cb callback,
>> + void *user_data);
>
> This I'd also like to be moved before messages_abort().
>
>> --
>> 1.7.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Slawomir Bochenski

Thanks for your valuable comments. I will correct it and send it as multiple
patches.

Divya
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2012-01-05 11:15:18

by Slawomir Bochenski

[permalink] [raw]
Subject: Re: [PATCH obexd] MAP: Add UpdateInbox Function

Hi!

On Thu, Jan 5, 2012 at 10:43 AM, Divya Yadav <[email protected]> wrote:
> UpdateInbox function allows remote device to initiate an update of
> the server's inbox, i.e. the Map server shall contact the network

Maybe better to call it "MSE inbox".

> to retrieve new messages if available. If the server does not allow
> the network update it shall answer with a 'Not implemented'
> error response.

More like "does not support" than "does not allow".

> ---
> ?plugins/mas.c ? ? ? ? ? ? ?| ? 32 +++++++++++++++++++++++++++++++-
> ?plugins/messages-dummy.c ? | ? ?7 +++++++
> ?plugins/messages-tracker.c | ? 10 +++++++++-
> ?plugins/messages.h ? ? ? ? | ? 14 ++++++++++++++
> ?4 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/plugins/mas.c b/plugins/mas.c
> index 3c3104c..639d0b6 100644
> --- a/plugins/mas.c
> +++ b/plugins/mas.c
> @@ -503,6 +503,36 @@ static void *message_open(const char *name, int oflag, mode_t mode,
> ? ? ? ? ? ? ? ?return mas;
> ?}
>
> +static void inbox_update_cb(void *session, int err, void *user_data)

Callbacks added previously are all following each other, before the
section with *_open functions, so I'd rather move this one up. Also
I'd rather call it update_inbox_cb(), to be consistent with the
typedef.

> +{
> + ? ? ? struct mas_session *mas = user_data;
> +
> + ? ? ? DBG("");
> +
> + ? ? ? mas->finished = TRUE;
> +
> + ? ? ? if (err < 0)
> + ? ? ? ? ? ? ? obex_object_set_io_flags(mas, G_IO_ERR, err);
> + ? ? ? else
> + ? ? ? ? ? ? ? obex_object_set_io_flags(mas, G_IO_OUT, 0);
> +}
> +
> +static void *message_update_open(const char *name, int oflag, mode_t mode,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void *driver_data, size_t *size, int *err)
> +{
> + ? ? ? struct mas_session *mas = driver_data;
> +
> + ? ? ? DBG("");
> +
> + ? ? ? *err = messages_update_inbox(mas->backend_data, inbox_update_cb, mas);
> +
> + ? ? ? if (*err < 0) {
> + ? ? ? ? ? ? ? mas->finished = TRUE;

No need for this. If open returns NULL, no read/write is going to be called.

> + ? ? ? ? ? ? ? return NULL;
> + ? ? ? } else
> + ? ? ? ? ? ? ? return mas;
> +}

This function is missing check for oflag value - this MIME type should
return error on GET.

> +
> ?static void *any_open(const char *name, int oflag, mode_t mode,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *driver_data, size_t *size, int *err)
> ?{
> @@ -628,7 +658,7 @@ static struct obex_mime_type_driver mime_message_update = {
> ? ? ? ?.target = MAS_TARGET,
> ? ? ? ?.target_size = TARGET_SIZE,
> ? ? ? ?.mimetype = "x-bt/MAP-messageUpdate",
> - ? ? ? .open = any_open,
> + ? ? ? .open = message_update_open,
> ? ? ? ?.close = any_close,
> ? ? ? ?.read = any_read,
> ? ? ? ?.write = any_write,
> diff --git a/plugins/messages-dummy.c b/plugins/messages-dummy.c
> index e96b13b..58f04f6 100644
> --- a/plugins/messages-dummy.c
> +++ b/plugins/messages-dummy.c
> @@ -173,3 +173,10 @@ int messages_get_message(void *session,
> ?void messages_abort(void *session)
> ?{
> ?}
> +
> +int messages_update_inbox(void *session,
> + ? ? ? ? ? ? ? messages_update_inbox_cb callback,
> + ? ? ? ? ? ? ? void *user_data)
> +{
> + ? ? ? return -EINVAL;
> +}

Put this before messages_abort().

> diff --git a/plugins/messages-tracker.c b/plugins/messages-tracker.c
> index 8f9b30a..6a68b5b 100644
> --- a/plugins/messages-tracker.c
> +++ b/plugins/messages-tracker.c
> @@ -237,7 +237,8 @@ int messages_set_folder(void *s, const char *name, gboolean cdup)
> ? ? ? ?return 0;
> ?}
>
> -static gboolean async_get_folder_listing(void *s) {
> +static gboolean async_get_folder_listing(void *s)
> +{

This belongs into a separate patch.

> ? ? ? ?struct session *session = s;
> ? ? ? ?gboolean count = FALSE;
> ? ? ? ?int folder_count = 0;
> @@ -324,3 +325,10 @@ int messages_get_message(void *session,
> ?void messages_abort(void *session)
> ?{
> ?}
> +
> +int messages_update_inbox(void *session,
> + ? ? ? ? ? ? ? messages_update_inbox_cb callback,
> + ? ? ? ? ? ? ? void *user_data)
> +{
> + ? ? ? return -EINVAL;
> +}

Same as for messages-dummy.c.

> diff --git a/plugins/messages.h b/plugins/messages.h
> index 6982edd..ea231a3 100644
> --- a/plugins/messages.h
> +++ b/plugins/messages.h
> @@ -273,3 +273,17 @@ int messages_get_message(void *session,
> ?* session: Backend session.
> ?*/
> ?void messages_abort(void *session);
> +
> +/* ?Informs Message Server to Update Inbox via network

One space after asterisk, dot at the end.

> + *
> + * session: Backend session.
> + * user_data: User data if any to be sent

Missing new line. Also missing dot at the end of second line.

> + * Callback shall be called for every update inbox request, where if any error
> + * should be reported to the remote end.

"shall be called once" maybe. Second part not very understandable. I
also guess you mean error during initiating inbox update, because the
update procedure itself should take place after UpdateInbox MAP
request returns.

> + */
> +typedef void (*messages_update_inbox_cb)(void *session, int err,
> + ? ? ? ? ? ? ? void *user_data);
> +
> +int messages_update_inbox(void *session,
> + ? ? ? ? ? ? ? messages_update_inbox_cb callback,
> + ? ? ? ? ? ? ? void *user_data);

This I'd also like to be moved before messages_abort().

> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html



--
Slawomir Bochenski