Return-Path: MIME-Version: 1.0 In-Reply-To: <1325756632-29335-1-git-send-email-divya.yadav@samsung.com> References: <1325756632-29335-1-git-send-email-divya.yadav@samsung.com> Date: Thu, 5 Jan 2012 12:15:18 +0100 Message-ID: Subject: Re: [PATCH obexd] MAP: Add UpdateInbox Function From: Slawomir Bochenski To: Divya Yadav Cc: linux-bluetooth@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi! On Thu, Jan 5, 2012 at 10:43 AM, Divya Yadav 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 majordomo@vger.kernel.org > More majordomo info at ?http://vger.kernel.org/majordomo-info.html -- Slawomir Bochenski