[PATCH obexd 1/3] map_ap.c: Add implementation for map_ap_get*

[PATCH obexd 1/3] map_ap.c: Add implementation for map_ap_get*

[Divya Yadav]

---
 src/map_ap.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/src/map_ap.c b/src/map_ap.c
index 54e3bcf..dd3ed6b 100644
--- a/src/map_ap.c
+++ b/src/map_ap.c
@@ -246,22 +246,72 @@ uint8_t *map_ap_encode(map_ap_t *ap, size_t *length)
 
 gboolean map_ap_get_u8(map_ap_t *ap, enum map_ap_tag tag, uint8_t *val)
 {
-	return FALSE;
+	struct ap_entry *entry;
+	int offset = find_ap_def_offset(tag);
+
+	if (offset < 0 || ap_defs[offset].type != APT_UINT8)
+		return FALSE;
+
+	entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
+
+	if (!entry)
+		return FALSE;
+
+	*val = entry->val.u8;
+
+	return TRUE;
 }
 
 gboolean map_ap_get_u16(map_ap_t *ap, enum map_ap_tag tag, uint16_t *val)
 {
-	return FALSE;
+	struct ap_entry *entry;
+	int offset = find_ap_def_offset(tag);
+
+	if (offset < 0 || ap_defs[offset].type != APT_UINT16)
+		return FALSE;
+
+	entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
+
+	if (!entry)
+		return FALSE;
+
+	*val = entry->val.u16;
+
+	return TRUE;
 }
 
 gboolean map_ap_get_u32(map_ap_t *ap, enum map_ap_tag tag, uint32_t *val)
 {
-	return FALSE;
+	struct ap_entry *entry;
+	int offset = find_ap_def_offset(tag);
+
+	if (offset < 0 || ap_defs[offset].type != APT_UINT32)
+		return FALSE;
+
+	entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
+
+	if (!entry)
+		return FALSE;
+
+	*val = entry->val.u32;
+
+	return TRUE;
 }
 
 const char *map_ap_get_string(map_ap_t *ap, enum map_ap_tag tag)
 {
-	return NULL;
+	struct ap_entry *entry;
+	int offset = find_ap_def_offset(tag);
+
+	if (offset < 0 || ap_defs[offset].type != APT_STR)
+		return NULL;
+
+	entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
+
+	if (!entry)
+		return NULL;
+
+	return g_strdup(entry->val.str);
 }
 
 gboolean map_ap_set_u8(map_ap_t *ap, enum map_ap_tag tag, uint8_t val)
-- 
1.7.0.4

[PATCH obexd 2/3] MAP: Implement application parameter parsing in get/put requests

[Divya Yadav]

---
 plugins/mas.c |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/plugins/mas.c b/plugins/mas.c
index 23b1823..ade28c5 100644
--- a/plugins/mas.c
+++ b/plugins/mas.c
@@ -40,6 +40,7 @@
 #include "manager.h"
 
 #include "messages.h"
+#include "map_ap.h"
 
 /* Channel number according to bluez doc/assigned-numbers.txt */
 #define MAS_CHANNEL	16
@@ -108,6 +109,7 @@ struct mas_session {
 	gboolean finished;
 	gboolean nth_call;
 	GString *buffer;
+	map_ap_t *ap;
 };
 
 static const uint8_t MAS_TARGET[TARGET_SIZE] = {
@@ -120,6 +122,8 @@ static void reset_request(struct mas_session *mas)
 		g_string_free(mas->buffer, TRUE);
 		mas->buffer = NULL;
 	}
+	map_ap_free(mas->ap);
+	mas->ap = NULL;
 
 	mas->nth_call = FALSE;
 	mas->finished = FALSE;
@@ -171,6 +175,8 @@ static int mas_get(struct obex_session *os, void *user_data)
 	const char *type = obex_get_type(os);
 	const char *name = obex_get_name(os);
 	int ret;
+	const uint8_t *buffer = NULL;
+	ssize_t rsize = 0;
 
 	DBG("GET: name %s type %s mas %p",
 			name, type, mas);
@@ -178,6 +184,20 @@ static int mas_get(struct obex_session *os, void *user_data)
 	if (type == NULL)
 		return -EBADR;
 
+	rsize = obex_get_apparam(os, &buffer);
+
+	if (rsize < 0) {
+		if (g_ascii_strcasecmp(type, "x-bt/message") == 0) {
+			ret = -EBADR;
+			goto failed;
+		}
+	} else {
+		mas->ap = map_ap_decode(buffer, rsize);
+		if (mas->ap == NULL) {
+			ret = -EBADR;
+			goto failed;
+		}
+	}
 	ret = obex_get_stream_start(os, name);
 	if (ret < 0)
 		goto failed;
@@ -196,12 +216,28 @@ static int mas_put(struct obex_session *os, void *user_data)
 	const char *type = obex_get_type(os);
 	const char *name = obex_get_name(os);
 	int ret;
+	const uint8_t *buffer = NULL;
+	ssize_t rsize = 0;
 
 	DBG("PUT: name %s type %s mas %p", name, type, mas);
 
 	if (type == NULL)
 		return -EBADR;
 
+	rsize = obex_get_apparam(os, &buffer);
+	if (rsize < 0) {
+		if (g_ascii_strcasecmp(type, "x-bt/messageStatus") == 0 ||
+			g_ascii_strcasecmp(type, "x-bt/message") == 0) {
+			ret = -EBADR;
+			goto failed;
+		}
+	} else {
+		mas->ap = map_ap_decode(buffer, rsize);
+		if (mas->ap == NULL) {
+			ret = -EBADR;
+			goto failed;
+		}
+	}
 	ret = obex_put_stream_start(os, name);
 	if (ret < 0)
 		goto failed;
-- 
1.7.0.4

[PATCH obexd 3/3] MAP: Add SetMessageStatus function

[Divya Yadav]

---
 plugins/mas.c              |   44 +++++++++++++++++++++++++++++++++++++++++++-
 plugins/messages-dummy.c   |    8 ++++++++
 plugins/messages-tracker.c |    8 ++++++++
 plugins/messages.h         |   20 ++++++++++++++++++++
 4 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/plugins/mas.c b/plugins/mas.c
index ade28c5..6e30abb 100644
--- a/plugins/mas.c
+++ b/plugins/mas.c
@@ -456,6 +456,20 @@ static void update_inbox_cb(void *session, int err, void *user_data)
 		obex_object_set_io_flags(mas, G_IO_OUT, 0);
 }
 
+static void message_status_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;
@@ -573,6 +587,34 @@ static void *message_update_open(const char *name, int oflag, mode_t mode,
 		return mas;
 }
 
+static void *message_status_open(const char *name, int oflag, mode_t mode,
+					void *driver_data, size_t *size,
+					int *err)
+
+{
+	struct mas_session *mas = driver_data;
+	uint8_t indicator;
+	uint8_t value;
+
+	 if (!(oflag & O_WRONLY)) {
+		*err = -EBADR;
+		return NULL;
+	 }
+
+	map_ap_get_u8(mas->ap, MAP_AP_STATUSINDICATOR, &indicator);
+	map_ap_get_u8(mas->ap, MAP_AP_STATUSVALUE, &value);
+
+	DBG("Indicator = %d \n value = %d\n", indicator, value);
+
+	*err = messages_set_message_status(mas->backend_data, name, indicator,
+						value, message_status_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)
 {
@@ -688,7 +730,7 @@ static struct obex_mime_type_driver mime_message_status = {
 	.target = MAS_TARGET,
 	.target_size = TARGET_SIZE,
 	.mimetype = "x-bt/messageStatus",
-	.open = any_open,
+	.open = message_status_open,
 	.close = any_close,
 	.read = any_read,
 	.write = any_write,
diff --git a/plugins/messages-dummy.c b/plugins/messages-dummy.c
index 511857e..67e335d 100644
--- a/plugins/messages-dummy.c
+++ b/plugins/messages-dummy.c
@@ -173,6 +173,14 @@ int messages_update_inbox(void *session, messages_update_inbox_cb callback,
 	return -ENOSYS;
 }
 
+int messages_set_message_status(void *session, const char *handle,
+					int indicator, int value,
+					messages_set_message_status_cb callback,
+					void *user_data)
+{
+	return -ENOSYS;
+}
+
 void messages_abort(void *session)
 {
 }
diff --git a/plugins/messages-tracker.c b/plugins/messages-tracker.c
index fb45210..f77b332 100644
--- a/plugins/messages-tracker.c
+++ b/plugins/messages-tracker.c
@@ -327,6 +327,14 @@ int messages_update_inbox(void *session, messages_update_inbox_cb callback,
 	return -ENOSYS;
 }
 
+int messages_set_message_status(void *session, const char *handle,
+					int indicator, int value,
+					messages_set_message_status_cb callback,
+					void *user_data)
+{
+	return -ENOSYS;
+}
+
 void messages_abort(void *session)
 {
 }
diff --git a/plugins/messages.h b/plugins/messages.h
index 00a040c..9cdc377 100644
--- a/plugins/messages.h
+++ b/plugins/messages.h
@@ -277,6 +277,26 @@ typedef void (*messages_update_inbox_cb)(void *session, int err,
 int messages_update_inbox(void *session, messages_update_inbox_cb callback,
 							void *user_data);
 
+/* Informs Message Server to modify status of the message.
+ *
+ * session: Backend session.
+ * handle: Unique identifier to the message.
+ * indicator: To indicate which status information is to be modified,
+ *	"readStatus" or "deleteStatus".
+ * value: To indicate the value of the indicator
+ *	read/unread for "readstatus" and deleted/undeleted for "deleteStatus".
+ * Callback shall be called for every message status update request
+ *	recieved from MCE.
+ * user_data: User data if any to be sent.
+ */
+typedef void (*messages_set_message_status_cb)(void *session, int err,
+		void *user_data);
+
+int messages_set_message_status(void *session, const char *handle,
+					int indicator, int value,
+					messages_set_message_status_cb callback,
+					void *user_data);
+
 /* Aborts currently pending request.
  *
  * session: Backend session.
-- 
1.7.0.4

Re: [PATCH obexd 1/3] map_ap.c: Add implementation for map_ap_get*

[Slawomir Bochenski]

Hi!

On Thu, Mar 8, 2012 at 9:44 AM, Divya Yadav <divya.yadav@samsung.com> wrote:
> ---
>  src/map_ap.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/src/map_ap.c b/src/map_ap.c
> index 54e3bcf..dd3ed6b 100644
> --- a/src/map_ap.c
> +++ b/src/map_ap.c
> @@ -246,22 +246,72 @@ uint8_t *map_ap_encode(map_ap_t *ap, size_t *length)
>
>  gboolean map_ap_get_u8(map_ap_t *ap, enum map_ap_tag tag, uint8_t *val)
>  {
> -       return FALSE;
> +       struct ap_entry *entry;
> +       int offset = find_ap_def_offset(tag);
> +
> +       if (offset < 0 || ap_defs[offset].type != APT_UINT8)
> +               return FALSE;
> +
> +       entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
> +
> +       if (!entry)
> +               return FALSE;
> +
> +       *val = entry->val.u8;
> +
> +       return TRUE;
>  }
>
>  gboolean map_ap_get_u16(map_ap_t *ap, enum map_ap_tag tag, uint16_t *val)
>  {
> -       return FALSE;
> +       struct ap_entry *entry;
> +       int offset = find_ap_def_offset(tag);
> +
> +       if (offset < 0 || ap_defs[offset].type != APT_UINT16)
> +               return FALSE;
> +
> +       entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
> +
> +       if (!entry)
> +               return FALSE;
> +
> +       *val = entry->val.u16;
> +
> +       return TRUE;
>  }
>
>  gboolean map_ap_get_u32(map_ap_t *ap, enum map_ap_tag tag, uint32_t *val)
>  {
> -       return FALSE;
> +       struct ap_entry *entry;
> +       int offset = find_ap_def_offset(tag);
> +
> +       if (offset < 0 || ap_defs[offset].type != APT_UINT32)
> +               return FALSE;
> +
> +       entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
> +
> +       if (!entry)
> +               return FALSE;
> +
> +       *val = entry->val.u32;
> +
> +       return TRUE;
>  }
>
>  const char *map_ap_get_string(map_ap_t *ap, enum map_ap_tag tag)
>  {
> -       return NULL;
> +       struct ap_entry *entry;
> +       int offset = find_ap_def_offset(tag);
> +
> +       if (offset < 0 || ap_defs[offset].type != APT_STR)
> +               return NULL;
> +
> +       entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
> +
> +       if (!entry)
> +               return NULL;
> +
> +       return g_strdup(entry->val.str);
>  }
>
>  gboolean map_ap_set_u8(map_ap_t *ap, enum map_ap_tag tag, uint8_t val)

Well, those are obviously quite right, as those are copies of
appropriate map_ap_set_* functions made by me, with the logic reversed
for reading. So almost exactly identical to what I have done already
and what I would send is some next series of patches (but now another
series are waiting to accepted). The only difference is that I do not
strdup string value in map_ap_get_string(), because there's no point
in it.

Patch no 2 is not how I did this and how I like it to be done.
Especially ugly are the checks for parameters presence performed
inside get and puts functions by string comparing MIME types. Those
comparison is already done for selecting mime driver so parameter
verification can be done in appropriate functions for those drivers.
So in this case e.g. in your message_status_open().

I don't know how this could work. I told you already that I also have
a code for setting message statuses or message parsing and uploading
and this will come next when browsing is fully accepted upstream. So
what is the point of reinventing the wheel, i.e. rewriting what is
already written and waiting to be upstreamed?

> --
> 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

Re: [PATCH obexd 1/3] map_ap.c: Add implementation for map_ap_get*

[girish.joshi]

Hello,

--------------------------------------------------
From: "Slawomir Bochenski" <lkslawek@gmail.com>
Sent: Thursday, March 08, 2012 2:57 PM
To: "Divya Yadav" <divya.yadav@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH obexd 1/3] map_ap.c: Add implementation for map_ap_get*

> Hi!
>
> On Thu, Mar 8, 2012 at 9:44 AM, Divya Yadav <divya.yadav@samsung.com> 
> wrote:
>> ---
>>  src/map_ap.c |   58 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>>  1 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/map_ap.c b/src/map_ap.c
>> index 54e3bcf..dd3ed6b 100644
>> --- a/src/map_ap.c
>> +++ b/src/map_ap.c
>> @@ -246,22 +246,72 @@ uint8_t *map_ap_encode(map_ap_t *ap, size_t 
>> *length)
>>
>>  gboolean map_ap_get_u8(map_ap_t *ap, enum map_ap_tag tag, uint8_t *val)
>>  {
>> -       return FALSE;
>> +       struct ap_entry *entry;
>> +       int offset = find_ap_def_offset(tag);
>> +
>> +       if (offset < 0 || ap_defs[offset].type != APT_UINT8)
>> +               return FALSE;
>> +
>> +       entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
>> +
>> +       if (!entry)
>> +               return FALSE;
>> +
>> +       *val = entry->val.u8;
>> +
>> +       return TRUE;
>>  }
>>
>>  gboolean map_ap_get_u16(map_ap_t *ap, enum map_ap_tag tag, uint16_t 
>> *val)
>>  {
>> -       return FALSE;
>> +       struct ap_entry *entry;
>> +       int offset = find_ap_def_offset(tag);
>> +
>> +       if (offset < 0 || ap_defs[offset].type != APT_UINT16)
>> +               return FALSE;
>> +
>> +       entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
>> +
>> +       if (!entry)
>> +               return FALSE;
>> +
>> +       *val = entry->val.u16;
>> +
>> +       return TRUE;
>>  }
>>
>>  gboolean map_ap_get_u32(map_ap_t *ap, enum map_ap_tag tag, uint32_t 
>> *val)
>>  {
>> -       return FALSE;
>> +       struct ap_entry *entry;
>> +       int offset = find_ap_def_offset(tag);
>> +
>> +       if (offset < 0 || ap_defs[offset].type != APT_UINT32)
>> +               return FALSE;
>> +
>> +       entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
>> +
>> +       if (!entry)
>> +               return FALSE;
>> +
>> +       *val = entry->val.u32;
>> +
>> +       return TRUE;
>>  }
>>
>>  const char *map_ap_get_string(map_ap_t *ap, enum map_ap_tag tag)
>>  {
>> -       return NULL;
>> +       struct ap_entry *entry;
>> +       int offset = find_ap_def_offset(tag);
>> +
>> +       if (offset < 0 || ap_defs[offset].type != APT_STR)
>> +               return NULL;
>> +
>> +       entry = g_hash_table_lookup(ap, GINT_TO_POINTER(tag));
>> +
>> +       if (!entry)
>> +               return NULL;
>> +
>> +       return g_strdup(entry->val.str);
>>  }
>>
>>  gboolean map_ap_set_u8(map_ap_t *ap, enum map_ap_tag tag, uint8_t val)
>
> Well, those are obviously quite right, as those are copies of
> appropriate map_ap_set_* functions made by me, with the logic reversed
> for reading. So almost exactly identical to what I have done already
> and what I would send is some next series of patches (but now another
> series are waiting to accepted). The only difference is that I do not
> strdup string value in map_ap_get_string(), because there's no point
> in it.
>
> Patch no 2 is not how I did this and how I like it to be done.
> Especially ugly are the checks for parameters presence performed
> inside get and puts functions by string comparing MIME types. Those
> comparison is already done for selecting mime driver so parameter
> verification can be done in appropriate functions for those drivers.
> So in this case e.g. in your message_status_open().
>
> I don't know how this could work. I told you already that I also have
> a code for setting message statuses or message parsing and uploading
> and this will come next when browsing is fully accepted upstream. So
> what is the point of reinventing the wheel, i.e. rewriting what is
> already written and waiting to be upstreamed?
>
>>
As per the discussion with you, we will wait for the code from you in the
obexd branch for some more time I hope your code will be applied soon.
As we discussed, we do understand that the code has to be reviewed
before applying it to the main branch, and as we discussed, we can try to
share code with some other way to avoid the duplicate work among the
developers.

Thanks,
Girish A J
>> 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
> --
> 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