2011-07-12 08:07:53

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH hcidump 1/2] Add check to verify AVRCP pdu length matches frame length

From: Luiz Augusto von Dentz <[email protected]>

---
parser/avrcp.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/parser/avrcp.c b/parser/avrcp.c
index 0fa38d1..43e8a8b 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -226,6 +226,11 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)

printf("AVRCP: %s: pt 0x%02x len 0x%04x\n", pdu2str(pduid), pt, len);

+ if (len != frm->len) {
+ p_indent(level, frm);
+ printf("PDU Malformed\n");
+ }
+
raw_dump(level, frm);
}

--
1.7.6



2011-07-12 12:32:44

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH hcidump 2/2] Add parsing for AVRCP GetCapabilities

Hi Lucas,

On Tue, Jul 12, 2011 at 3:01 PM, Lucas De Marchi
<[email protected]> wrote:
> On Tue, Jul 12, 2011 at 5:07 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> ---
>> ?parser/avrcp.c | ? 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> ?1 files changed, 57 insertions(+), 1 deletions(-)
>>
>> diff --git a/parser/avrcp.c b/parser/avrcp.c
>> index 43e8a8b..9529b33 100644
>> --- a/parser/avrcp.c
>> +++ b/parser/avrcp.c
>> @@ -213,6 +213,54 @@ static const char *pdu2str(uint8_t pduid)
>> ? ? ? ?}
>> ?}
>>
>> +static char *cap2str(uint8_t cap)
>> +{
>> + ? ? ? switch (cap) {
>> + ? ? ? case 0x2:
>> + ? ? ? ? ? ? ? return "CompanyID";
>> + ? ? ? case 0x3:
>> + ? ? ? ? ? ? ? return "EventsID";
>> + ? ? ? default:
>> + ? ? ? ? ? ? ? return "Unknown";
>> + ? ? ? }
>> +}
>> +
>> +static void avrcp_get_capabilities_dump(int level, struct frame *frm, uint16_t len)
>> +{
>> + ? ? ? uint8_t cap;
>> + ? ? ? uint8_t count;
>> +
>> + ? ? ? p_indent(level, frm);
>> +
>> + ? ? ? if (len < 1) {
>> + ? ? ? ? ? ? ? printf("PDU Malformed\n");
>> + ? ? ? ? ? ? ? raw_dump(level, frm);
>> + ? ? ? ? ? ? ? return;
>> + ? ? ? }
>> +
>> + ? ? ? cap = get_u8(frm);
>> + ? ? ? printf("Capability ID: 0x%02x (%s)\n", cap, cap2str(cap));
>> +
>> + ? ? ? if (len == 1)
>> + ? ? ? ? ? ? ? return;
>> +
>> + ? ? ? p_indent(level, frm);
>> +
>> + ? ? ? count = get_u8(frm);
>> + ? ? ? printf("Capability Count: 0x%02x\n", count);
>> +
>> + ? ? ? for (; count > 0; count--) {
>> + ? ? ? ? ? ? ? int i;
>> +
>> + ? ? ? ? ? ? ? p_indent(level, frm);
>> +
>> + ? ? ? ? ? ? ? printf("%s: 0x", cap2str(cap));
>> + ? ? ? ? ? ? ? for (i = 0; i < 3; i++)
>> + ? ? ? ? ? ? ? ? ? ? ? printf("0x%02x", get_u8(frm));
>
> Whouldn't it be better to print the company id as a whole instead of split it?

Its a bug, Im actually following the spec examples how to display the
company ids e.g 0xXXXXXX

> Also, if it is a response to EVENTS_SUPPORTED, this field is not
> 3-byte long (table 5.5 of AVRCP 1.3).

Yep, I was planning to add it latter but I guess it makes more to
sense add only complete pdu parsers, btw lets use the latest adopted
spec (1.4) as reference.



--
Luiz Augusto von Dentz

2011-07-12 12:01:00

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH hcidump 2/2] Add parsing for AVRCP GetCapabilities

On Tue, Jul 12, 2011 at 5:07 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> ---
> ?parser/avrcp.c | ? 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> ?1 files changed, 57 insertions(+), 1 deletions(-)
>
> diff --git a/parser/avrcp.c b/parser/avrcp.c
> index 43e8a8b..9529b33 100644
> --- a/parser/avrcp.c
> +++ b/parser/avrcp.c
> @@ -213,6 +213,54 @@ static const char *pdu2str(uint8_t pduid)
> ? ? ? ?}
> ?}
>
> +static char *cap2str(uint8_t cap)
> +{
> + ? ? ? switch (cap) {
> + ? ? ? case 0x2:
> + ? ? ? ? ? ? ? return "CompanyID";
> + ? ? ? case 0x3:
> + ? ? ? ? ? ? ? return "EventsID";
> + ? ? ? default:
> + ? ? ? ? ? ? ? return "Unknown";
> + ? ? ? }
> +}
> +
> +static void avrcp_get_capabilities_dump(int level, struct frame *frm, uint16_t len)
> +{
> + ? ? ? uint8_t cap;
> + ? ? ? uint8_t count;
> +
> + ? ? ? p_indent(level, frm);
> +
> + ? ? ? if (len < 1) {
> + ? ? ? ? ? ? ? printf("PDU Malformed\n");
> + ? ? ? ? ? ? ? raw_dump(level, frm);
> + ? ? ? ? ? ? ? return;
> + ? ? ? }
> +
> + ? ? ? cap = get_u8(frm);
> + ? ? ? printf("Capability ID: 0x%02x (%s)\n", cap, cap2str(cap));
> +
> + ? ? ? if (len == 1)
> + ? ? ? ? ? ? ? return;
> +
> + ? ? ? p_indent(level, frm);
> +
> + ? ? ? count = get_u8(frm);
> + ? ? ? printf("Capability Count: 0x%02x\n", count);
> +
> + ? ? ? for (; count > 0; count--) {
> + ? ? ? ? ? ? ? int i;
> +
> + ? ? ? ? ? ? ? p_indent(level, frm);
> +
> + ? ? ? ? ? ? ? printf("%s: 0x", cap2str(cap));
> + ? ? ? ? ? ? ? for (i = 0; i < 3; i++)
> + ? ? ? ? ? ? ? ? ? ? ? printf("0x%02x", get_u8(frm));

Whouldn't it be better to print the company id as a whole instead of split it?

Also, if it is a response to EVENTS_SUPPORTED, this field is not
3-byte long (table 5.5 of AVRCP 1.3).


Lucas De Marchi

2011-07-12 08:07:54

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH hcidump 2/2] Add parsing for AVRCP GetCapabilities

From: Luiz Augusto von Dentz <[email protected]>

---
parser/avrcp.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 57 insertions(+), 1 deletions(-)

diff --git a/parser/avrcp.c b/parser/avrcp.c
index 43e8a8b..9529b33 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -213,6 +213,54 @@ static const char *pdu2str(uint8_t pduid)
}
}

+static char *cap2str(uint8_t cap)
+{
+ switch (cap) {
+ case 0x2:
+ return "CompanyID";
+ case 0x3:
+ return "EventsID";
+ default:
+ return "Unknown";
+ }
+}
+
+static void avrcp_get_capabilities_dump(int level, struct frame *frm, uint16_t len)
+{
+ uint8_t cap;
+ uint8_t count;
+
+ p_indent(level, frm);
+
+ if (len < 1) {
+ printf("PDU Malformed\n");
+ raw_dump(level, frm);
+ return;
+ }
+
+ cap = get_u8(frm);
+ printf("Capability ID: 0x%02x (%s)\n", cap, cap2str(cap));
+
+ if (len == 1)
+ return;
+
+ p_indent(level, frm);
+
+ count = get_u8(frm);
+ printf("Capability Count: 0x%02x\n", count);
+
+ for (; count > 0; count--) {
+ int i;
+
+ p_indent(level, frm);
+
+ printf("%s: 0x", cap2str(cap));
+ for (i = 0; i < 3; i++)
+ printf("0x%02x", get_u8(frm));
+ printf("\n");
+ }
+}
+
static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
{
uint8_t pduid, pt;
@@ -229,9 +277,17 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
if (len != frm->len) {
p_indent(level, frm);
printf("PDU Malformed\n");
+ raw_dump(level, frm);
+ return;
}

- raw_dump(level, frm);
+ switch (pduid) {
+ case AVRCP_GET_CAPABILITIES:
+ avrcp_get_capabilities_dump(level + 1, frm, len);
+ break;
+ default:
+ raw_dump(level, frm);
+ }
}

static char *op2str(uint8_t op)
--
1.7.6