Changes made to handle the Extended Event Report 1.1
1) In SDP, the MAP supported featured bit is updated.
2) In map-event, corresponding new event type and the new attribute introduced in Event report 1.1 is added.
3) In mns corresponding handlers are added.
First two patches related to MAP 1.2 implementation.
Rest are all simple issues.
---
lib/sdp.h | 2 +-
obexd/client/map-event.h | 7 ++++++-
obexd/client/mns.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/lib/sdp.h b/lib/sdp.h
index cc10e9f..76f61e1 100644
--- a/lib/sdp.h
+++ b/lib/sdp.h
@@ -306,7 +306,7 @@ extern "C" {
#define SDP_ATTR_MAS_INSTANCE_ID 0x0315
#define SDP_ATTR_SUPPORTED_MESSAGE_TYPES 0x0316
#define SDP_ATTR_PBAP_SUPPORTED_FEATURES 0x0317
-#define SDP_ATTR_MAP_SUPPORTED_FEATURES 0x0317
+#define SDP_ATTR_MAP_SUPPORTED_FEATURES 0x031f
#define SDP_ATTR_SPECIFICATION_ID 0x0200
#define SDP_ATTR_VENDOR_ID 0x0201
diff --git a/obexd/client/map-event.h b/obexd/client/map-event.h
index ba5d5d2..99cb0c2 100644
--- a/obexd/client/map-event.h
+++ b/obexd/client/map-event.h
@@ -32,7 +32,8 @@ enum map_event_type {
MAP_ET_MEMORY_FULL,
MAP_ET_MEMORY_AVAILABLE,
MAP_ET_MESSAGE_DELETED,
- MAP_ET_MESSAGE_SHIFT
+ MAP_ET_MESSAGE_SHIFT,
+ MAP_ET_READ_STATUS_CHANGED
};
struct map_event {
@@ -41,6 +42,10 @@ struct map_event {
char *folder;
char *old_folder;
char *msg_type;
+ char *datetime;
+ char *subject;
+ char *sender_name;
+ char *priority;
};
/* Handle notification in map client.
diff --git a/obexd/client/mns.c b/obexd/client/mns.c
index d638886..8087933 100644
--- a/obexd/client/mns.c
+++ b/obexd/client/mns.c
@@ -180,6 +180,8 @@ static void parse_event_report_type(struct map_event *event, const char *value)
event->type = MAP_ET_MESSAGE_DELETED;
else if (!g_ascii_strcasecmp(value, "MessageShift"))
event->type = MAP_ET_MESSAGE_SHIFT;
+ else if (!g_ascii_strcasecmp(value, "ReadStatusChanged"))
+ event->type = MAP_ET_READ_STATUS_CHANGED;
}
static void parse_event_report_handle(struct map_event *event,
@@ -218,6 +220,30 @@ static void parse_event_report_msg_type(struct map_event *event,
event->msg_type = g_strdup(value);
}
+static void parse_event_report_date_time(struct map_event *event,
+ const char *value)
+{
+ event->datetime = g_strdup(value);
+}
+
+static void parse_event_report_subject(struct map_event *event,
+ const char *value)
+{
+ event->subject = g_strdup(value);
+}
+
+static void parse_event_report_sender_name(struct map_event *event,
+ const char *value)
+{
+ event->sender_name = g_strdup(value);
+}
+
+static void parse_event_report_priority(struct map_event *event,
+ const char *value)
+{
+ event->priority = g_strdup(value);
+}
+
static struct map_event_report_parser {
const char *name;
void (*func) (struct map_event *event, const char *value);
@@ -227,6 +253,10 @@ static struct map_event_report_parser {
{ "folder", parse_event_report_folder },
{ "old_folder", parse_event_report_old_folder },
{ "msg_type", parse_event_report_msg_type },
+ { "datetime", parse_event_report_date_time },
+ { "subject", parse_event_report_subject },
+ { "sender_name", parse_event_report_sender_name },
+ { "priority", parse_event_report_priority },
{ }
};
--
1.9.1
Hi Gowtham,
On Thursday 11 of September 2014 18:50:07 Gowtham Anandha Babu wrote:
Commit header should be like:
android/hal-utils: Fix null pointer dereference
> Handles the possible null pointer dereference: bd_addr by checking it
> against null. ---
Commit body should be limited to 72 characters.
> android/hal-utils.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/android/hal-utils.c b/android/hal-utils.c
> index ceefefc..64ab5a1 100644
> --- a/android/hal-utils.c
> +++ b/android/hal-utils.c
> @@ -166,11 +166,13 @@ int int2str_findstr(const char *str, const struct
> int2str m[]) */
> const char *bt_bdaddr_t2str(const bt_bdaddr_t *bd_addr, char *buf)
> {
> - const uint8_t *p = bd_addr->address;
> + const uint8_t *p;
>
> if (!bd_addr)
> return strcpy(buf, "NULL");
>
> + p = bd_addr->address;
> +
> snprintf(buf, MAX_ADDR_STR_LEN, "%02x:%02x:%02x:%02x:%02x:%02x",
> p[0], p[1], p[2], p[3], p[4], p[5]);
I've fixed those minors myself and pushed the patch, but please keep it in
mind for any future submissions. Thanks.
--
BR
Szymon Janc
Hi,
> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:[email protected]]
> Sent: Friday, September 12, 2014 2:16 AM
> To: Gowtham Anandha Babu; [email protected]; Dmitry
> Kasatkin; Bharat Panda; [email protected]
> Subject: Re: [PATCH 1/7] obexd/client : Handle the MAP Extended Event
> Report 1.1
>
> Hi,
>
> On Thu, Sep 11, 2014 at 7:37 PM, Johan Hedberg
> <[email protected]> wrote:
> > Hi,
> >
> > On Thu, Sep 11, 2014, Gowtham Anandha Babu wrote:
> >> struct map_event {
> >> @@ -41,6 +42,10 @@ struct map_event {
> >> char *folder;
> >> char *old_folder;
> >> char *msg_type;
> >> + char *datetime;
> >> + char *subject;
> >> + char *sender_name;
> >> + char *priority;
> >> };
> > <snip>
> >> +static void parse_event_report_date_time(struct map_event *event,
> >> + const char
> >> +*value) {
> >> + event->datetime = g_strdup(value); }
> >> +
> >> +static void parse_event_report_subject(struct map_event *event,
> >> + const char
> >> +*value) {
> >> + event->subject = g_strdup(value); }
> >> +
> >> +static void parse_event_report_sender_name(struct map_event
> *event,
> >> + const char
> >> +*value) {
> >> + event->sender_name = g_strdup(value); }
> >> +
> >> +static void parse_event_report_priority(struct map_event *event,
> >> + const char
> >> +*value) {
> >> + event->priority = g_strdup(value); }
> >
> > Where are all these freed? Don't you need to update the
> > map_event_free() function? If you're not yet doing so, please always
> > test your code with "valgrind --leak-check=full".
>
> And it is mentioned in the HACKING document.
>
> --
> Luiz Augusto von Dentz
I had sent the updated patch with freeing all the event attributes and tested with valgrind(No leak). Please have a look at it.
Regards,
Gowtham Anandha Babu
Hi,
> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:[email protected]]
> Sent: Friday, September 12, 2014 2:30 AM
> To: Gowtham Anandha Babu
> Cc: [email protected]; Dmitry Kasatkin; Bharat Panda;
> [email protected]
> Subject: Re: [PATCH 2/7] obexd/client/map : Handle MAP
> ReadStatusChanged event type
>
> Hi,
>
> On Thu, Sep 11, 2014 at 4:20 PM, Gowtham Anandha Babu
> <[email protected]> wrote:
> > Adds ReadStatusChanged MCE event type handling in
> > map_handle_notification()
> > ---
> > obexd/client/map.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/obexd/client/map.c b/obexd/client/map.c index
> > 520e492..a505707 100644
> > --- a/obexd/client/map.c
> > +++ b/obexd/client/map.c
> > @@ -1872,6 +1872,25 @@ static void map_handle_status_changed(struct
> map_data *map,
> >
> > "Status"); }
> >
> > +static void map_handle_read_status_changed(struct map_data *map,
> > + struct
> > +map_event *event) {
> > + struct map_msg *msg;
> > +
> > + msg = g_hash_table_lookup(map->messages, &event->handle);
> > + if (msg == NULL)
> > + return;
> > +
> > + /* In MAP V 1.2 specification : ReadStatusChanged event didn't have
> clear explaination.
> > + So as of now it will set the message read status as "yes" if it was "no"
> already
> > + and the other way round. This implementation may change once
> > + we get 'read' attribute in the event report. */
>
> The coding style for multi-line comment is wrong, please check our coding
> style there is a specific chapter for it, and to be honest I did not get it why we
> are toggling the value if it is not clearly explained, btw there is a typo there,
> perhaps you should check against the test specification if there is any test
> regarding that or any errata documenting the behavior.
>
> > + if(msg->flags & MAP_MSG_FLAG_READ)
> > + parse_read(msg,"no");
> > + else
> > + parse_read(msg,"yes"); }
> > +
> > static void map_handle_folder_changed(struct map_data *map,
> > struct map_event *event,
> > const char
> > *folder) @@ -1927,6 +1946,9 @@ static void
> map_handle_notification(struct map_event *event, void *user_data)
> > case MAP_ET_MESSAGE_SHIFT:
> > map_handle_folder_changed(map, event, event->folder);
> > break;
> > + case MAP_ET_READ_STATUS_CHANGED:
> > + map_handle_read_status_changed(map, event);
> > + break;
> > default:
> > break;
> > }
> > --
> > 1.9.1
> >
> > --
> > 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
>
>
>
> --
> Luiz Augusto von Dentz
I will send the updated patch with proper multi-line comment following the coding style and with modification(if any).
Btw,
As per the specification: Pg.no : 35 on MAP 1.2 spec
"ReadStatusChanged: indicates that the 'read' status of a message (see 3.1.6) has
been changed on the MSE. "
But the actual Event Report which was provided as test case in PTS tool was:
<MAP-event-report version = "1.1">
<event type = "ReadStatusChanged" handle = "12345678" folder =
"TELECOM/MSG/INBOX" msg_type = "SMS_CDMA" subject = "Hello" datetime =
"20110221T130510" sender_name = "Jamie" priority = "yes" />
</MAP-event-report>
>From the above event report 1.1, we cannot retrieve the read status of a message.
ReadStatusChanged event is received by the MCE only if the previous read status has been changed in the MSE device.
There are two approaches we may follow:
1) We have to toggle the read status whenever we get the ReadstatusChanged event.
2) Or simply we can call parse_read() function without toggling as show below, because the only way read status can change is from unread to read.
Instead of if else , we can have a single line implementation :
parse_read(msg,"yes");
What do you think?
Regards,
Gowtham Anandha Babu
Hi,
On Thu, Sep 11, 2014 at 4:20 PM, Gowtham Anandha Babu
<[email protected]> wrote:
> Adds ReadStatusChanged MCE event type handling in map_handle_notification()
> ---
> obexd/client/map.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/obexd/client/map.c b/obexd/client/map.c
> index 520e492..a505707 100644
> --- a/obexd/client/map.c
> +++ b/obexd/client/map.c
> @@ -1872,6 +1872,25 @@ static void map_handle_status_changed(struct map_data *map,
> "Status");
> }
>
> +static void map_handle_read_status_changed(struct map_data *map,
> + struct map_event *event)
> +{
> + struct map_msg *msg;
> +
> + msg = g_hash_table_lookup(map->messages, &event->handle);
> + if (msg == NULL)
> + return;
> +
> + /* In MAP V 1.2 specification : ReadStatusChanged event didn't have clear explaination.
> + So as of now it will set the message read status as "yes" if it was "no" already
> + and the other way round. This implementation may change once we get 'read' attribute in the event report. */
The coding style for multi-line comment is wrong, please check our
coding style there is a specific chapter for it, and to be honest I
did not get it why we are toggling the value if it is not clearly
explained, btw there is a typo there, perhaps you should check against
the test specification if there is any test regarding that or any
errata documenting the behavior.
> + if(msg->flags & MAP_MSG_FLAG_READ)
> + parse_read(msg,"no");
> + else
> + parse_read(msg,"yes");
> +}
> +
> static void map_handle_folder_changed(struct map_data *map,
> struct map_event *event,
> const char *folder)
> @@ -1927,6 +1946,9 @@ static void map_handle_notification(struct map_event *event, void *user_data)
> case MAP_ET_MESSAGE_SHIFT:
> map_handle_folder_changed(map, event, event->folder);
> break;
> + case MAP_ET_READ_STATUS_CHANGED:
> + map_handle_read_status_changed(map, event);
> + break;
> default:
> break;
> }
> --
> 1.9.1
>
> --
> 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
--
Luiz Augusto von Dentz
Hi,
On Thu, Sep 11, 2014 at 7:37 PM, Johan Hedberg <[email protected]> wrote:
> Hi,
>
> On Thu, Sep 11, 2014, Gowtham Anandha Babu wrote:
>> struct map_event {
>> @@ -41,6 +42,10 @@ struct map_event {
>> char *folder;
>> char *old_folder;
>> char *msg_type;
>> + char *datetime;
>> + char *subject;
>> + char *sender_name;
>> + char *priority;
>> };
> <snip>
>> +static void parse_event_report_date_time(struct map_event *event,
>> + const char *value)
>> +{
>> + event->datetime = g_strdup(value);
>> +}
>> +
>> +static void parse_event_report_subject(struct map_event *event,
>> + const char *value)
>> +{
>> + event->subject = g_strdup(value);
>> +}
>> +
>> +static void parse_event_report_sender_name(struct map_event *event,
>> + const char *value)
>> +{
>> + event->sender_name = g_strdup(value);
>> +}
>> +
>> +static void parse_event_report_priority(struct map_event *event,
>> + const char *value)
>> +{
>> + event->priority = g_strdup(value);
>> +}
>
> Where are all these freed? Don't you need to update the map_event_free()
> function? If you're not yet doing so, please always test your code with
> "valgrind --leak-check=full".
And it is mentioned in the HACKING document.
--
Luiz Augusto von Dentz
Hi,
On Thu, Sep 11, 2014, Gowtham Anandha Babu wrote:
> struct map_event {
> @@ -41,6 +42,10 @@ struct map_event {
> char *folder;
> char *old_folder;
> char *msg_type;
> + char *datetime;
> + char *subject;
> + char *sender_name;
> + char *priority;
> };
<snip>
> +static void parse_event_report_date_time(struct map_event *event,
> + const char *value)
> +{
> + event->datetime = g_strdup(value);
> +}
> +
> +static void parse_event_report_subject(struct map_event *event,
> + const char *value)
> +{
> + event->subject = g_strdup(value);
> +}
> +
> +static void parse_event_report_sender_name(struct map_event *event,
> + const char *value)
> +{
> + event->sender_name = g_strdup(value);
> +}
> +
> +static void parse_event_report_priority(struct map_event *event,
> + const char *value)
> +{
> + event->priority = g_strdup(value);
> +}
Where are all these freed? Don't you need to update the map_event_free()
function? If you're not yet doing so, please always test your code with
"valgrind --leak-check=full".
Johan
Handles the possible null pointer dereference: bd_addr by checking it against null.
---
android/hal-utils.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/android/hal-utils.c b/android/hal-utils.c
index ceefefc..64ab5a1 100644
--- a/android/hal-utils.c
+++ b/android/hal-utils.c
@@ -166,11 +166,13 @@ int int2str_findstr(const char *str, const struct int2str m[])
*/
const char *bt_bdaddr_t2str(const bt_bdaddr_t *bd_addr, char *buf)
{
- const uint8_t *p = bd_addr->address;
+ const uint8_t *p;
if (!bd_addr)
return strcpy(buf, "NULL");
+ p = bd_addr->address;
+
snprintf(buf, MAX_ADDR_STR_LEN, "%02x:%02x:%02x:%02x:%02x:%02x",
p[0], p[1], p[2], p[3], p[4], p[5]);
--
1.9.1
Fix the syntax error.
---
tools/hciattach.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hciattach.c b/tools/hciattach.c
index 1904ac5..db68b51 100644
--- a/tools/hciattach.c
+++ b/tools/hciattach.c
@@ -128,7 +128,7 @@ int uart_speed(int s)
return B3500000;
#endif
#ifdef B3710000
- case 3710000
+ case 3710000:
return B3710000;
#endif
#ifdef B4000000
--
1.9.1
Fix the usage of same expression on both sides of '||' in if
---
tools/seq2bseq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/seq2bseq.c b/tools/seq2bseq.c
index 7657a57..521d20e 100644
--- a/tools/seq2bseq.c
+++ b/tools/seq2bseq.c
@@ -40,7 +40,7 @@ static int convert_line(int fd, const char *line)
char str[3];
unsigned char val;
- if (line[0] == '*' || line[0] == '\r' || line[0] == '\r')
+ if (line[0] == '*' || line[0] == '\r')
return 0;
while (1) {
--
1.9.1
Handles resource leak.
---
tools/csr_usb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/csr_usb.c b/tools/csr_usb.c
index 5fb6bdc..a1d7324 100644
--- a/tools/csr_usb.c
+++ b/tools/csr_usb.c
@@ -80,9 +80,12 @@ static int read_value(const char *name, const char *attr, const char *format)
return -1;
n = fscanf(file, format, &value);
- if (n != 1)
+ if (n != 1) {
+ fclose(file);
return -1;
+ }
+ fclose(file);
return value;
}
--
1.9.1
The Variable 'written' is reassigned a value before the old one has been used.
The below on fix the same.
---
tools/btsnoop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/btsnoop.c b/tools/btsnoop.c
index 6ca62d2..86f4691 100644
--- a/tools/btsnoop.c
+++ b/tools/btsnoop.c
@@ -211,7 +211,7 @@ next_packet:
goto next_packet;
}
- written = input_pkt[select_input].size = htobe32(toread - 1);
+ input_pkt[select_input].size = htobe32(toread - 1);
written = input_pkt[select_input].len = htobe32(toread - 1);
switch (buf[0]) {
--
1.9.1
Adds ReadStatusChanged MCE event type handling in map_handle_notification()
---
obexd/client/map.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 520e492..a505707 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -1872,6 +1872,25 @@ static void map_handle_status_changed(struct map_data *map,
"Status");
}
+static void map_handle_read_status_changed(struct map_data *map,
+ struct map_event *event)
+{
+ struct map_msg *msg;
+
+ msg = g_hash_table_lookup(map->messages, &event->handle);
+ if (msg == NULL)
+ return;
+
+ /* In MAP V 1.2 specification : ReadStatusChanged event didn't have clear explaination.
+ So as of now it will set the message read status as "yes" if it was "no" already
+ and the other way round. This implementation may change once we get 'read' attribute in the event report. */
+
+ if(msg->flags & MAP_MSG_FLAG_READ)
+ parse_read(msg,"no");
+ else
+ parse_read(msg,"yes");
+}
+
static void map_handle_folder_changed(struct map_data *map,
struct map_event *event,
const char *folder)
@@ -1927,6 +1946,9 @@ static void map_handle_notification(struct map_event *event, void *user_data)
case MAP_ET_MESSAGE_SHIFT:
map_handle_folder_changed(map, event, event->folder);
break;
+ case MAP_ET_READ_STATUS_CHANGED:
+ map_handle_read_status_changed(map, event);
+ break;
default:
break;
}
--
1.9.1